[ActionList] Add hover to aria-current elements and other micro interactions
#1870
Conversation
Also included a quick transition for subGroup elements
Also: - Avoided sticky hover on touch devices with hover:hover media query - removed pointer: fine specificity to hover media queries - simplified reduce-motion media query
🦋 Changeset detectedLatest commit: cca1f62 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| &:hover { | ||
| cursor: pointer; | ||
| background-color: var(--color-action-list-item-default-hover-bg); | ||
| @media (hover: hover) { |
There was a problem hiding this comment.
I haven't tested this recently, but a while back using @media (hover: hover) was not really reliable for "hybrid devices" (like a laptop with a touch screen, or connecting a mouse to an iPad). The @media (hover: hover) would only work for what the browser thought is the "primary" input but would not adjust when switching touch/mouse on the fly. Maybe that improved by now and it is a bit of an edge case, so maybe still fine to keep it until we hear some reports about it. 🤷
There was a problem hiding this comment.
@simurai that's a great point! I think this is still an expected behavior for hybrid devices, according to the latest W3C Media Query specs.
Example 44
For example, on a touch screen device that can also be controlled by an optional mouse, the hover media feature should match hover: none, as the primary pointing device (the touch screen) does not allow the user to hover.
However, despite this, the optional mouse does allow users to hover. Authors should therefore be careful not to assume that the ':hover' pseudo class will never match on a device where 'hover:none' is true, but they should design layouts that do not depend on hovering to be fully usable.
I think the current solution is a good compromise. Showing or not showing a hover state (in case of a laptop with touch screen, or a touch device with a plugged-in mouse, respectively) is not an unexpected behavior for these non-primary types of interactions, I'd say.
👍
| @@ -300,11 +335,10 @@ | |||
| color: var(--color-danger-fg); | |||
| } | |||
|
|
|||
| @media (hover: hover) and (pointer: fine) { | |||
There was a problem hiding this comment.
Did you remove and (pointer: fine) because we weren't getting hover styles on touch devices with an optional mouse?
There was a problem hiding this comment.
@mperrotti no... I removed it because the pointer query shouldn't influence whether we display these :hover styles or not. Devices that have pointer: coarse which also support hover: hover should still display it. See the combination table from W3C for reference.
✌️
| @@ -154,11 +183,14 @@ | |||
| } | |||
|
|
|||
| .ActionList--subGroup { | |||
| display: block; | |||
| height: auto; | |||
This PR started as a quick fix for https://github.com/github/github/discussions/201185#discussioncomment-1907903 (private for hubbers), but I've also touched a couple of other related micro interactions:
Add hover background for selected item (fixes https://github.com/github/github/discussions/201185#discussioncomment-1907903)
Makes
-webkit-tap-highlight-colortransparent — this removes a blueish highlight background on touch devices. It was particularly problematic on items withsubGroupelements. Since:activestates remain available on touch, users should see the regular gray background instead.Preview
Add
touch-action: manipulation. This should speed up the tap response on touch devices. From MDN:Wrap
:hoverstyles around@media (hover: hover) { }media queries, to avoid sticky hovers on touch.Add "slide down" animation when expanding
subGroup. This should hopefully improve the parsing when interacting with longer lists, and compensate the user expectation since we're no longer redirecting to the first sub-item right away.Disable
max-nesting-depthandselector-max-specificitystylelint rules in the file level, as, per design, the component requires more nuanced CSS selectors throughout.Are additional changes needed?