feat(useFocusZone): update active-descendant on mousemove #1308
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 2f3567d 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 |
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/primer/primer-components/9KA5iYMtZmnZ9ZWbQBqsrFAv9ZFg |
dgreif
force-pushed
the
active-descendant-refinements
branch
from
fc7dfd8 to
4db22de
Compare
dgreif
force-pushed
the
active-descendant-refinements
branch
from
4db22de to
c59b206
Compare
dgreif
force-pushed
the
active-descendant-refinements
branch
from
c59b206 to
f4ff6d3
Compare
dgreif
force-pushed
the
active-descendant-refinements
branch
from
f4ff6d3 to
b92ac61
Compare
dgreif
force-pushed
the
active-descendant-refinements
branch
from
23313d6 to
6304616
Compare
dgreif
force-pushed
the
active-descendant-refinements
branch
from
6304616 to
ce5712c
Compare
dgreif
force-pushed
the
active-descendant-refinements
branch
from
ce5712c to
967f51d
Compare
dgreif
force-pushed
the
active-descendant-refinements
branch
from
967f51d to
b98d2da
Compare
vdepizzol
reviewed
Jun 22, 2021
vdepizzol
reviewed
Jun 22, 2021
vdepizzol
reviewed
Jun 22, 2021
Co-authored-by: Vinicius Depizzol <[email protected]>
smockle
reviewed
Jun 23, 2021
smockle
reviewed
Jun 23, 2021
smockle
reviewed
Jun 23, 2021
smockle
reviewed
Jun 23, 2021
smockle
reviewed
Jun 23, 2021
smockle
approved these changes
Jun 24, 2021
Merged
colebemis
pushed a commit
that referenced
this pull request
Jul 19, 2021
colebemis
added a commit
that referenced
this pull request
Jul 19, 2021
* add utility props to Box * update box docs * export box props * update snapshots * Create green-worms-nail.md * AvatarStack story in storybook * Update .changeset/green-worms-nail.md Co-authored-by: Cole Bemis <[email protected]> * Update docs/content/Box.md Co-authored-by: Cole Bemis <[email protected]> * Remove duplicate border system prop definitions * Remove duplicate grid system props definitions * Update FlexProps definition * Remove duplicate position system prop definitions * Update Box documentation * Update BoxProps * Update Box docs * Update Box props * fix: Type 'DropdownButton' as 'button' (#1318) * fix: Type 'DropdownButton' as 'button' * chore: Update snapshots * chore: Set test directory via config rather than flag (#1319) * feat(useFocusZone): update active-descendant on mousemove (#1308) * fix: Split `<Item>` labels (#1320) * fix: Separate 'Item' content into 'label' and 'description' * fix: Only add 'aria-describedby' when 'description' exists * fix: Memoize 'id' so 'Item's and labels match * fix: Don’t rely on 'id' which is possibly not globally-unique * fix: Restore semi-full-width 'Item' dividers, without giving up the semantic nesting. By “semantic nesting”, I mean that the 'Item' label and description are now siblings, which is preferable to the previous implementation, where the description node was a child of the label node. As a general principle, we should align DOM hierarchies with information hierarchies. An analogy: If I were using a bulleted list to describe a dog, I would not indent its breed as a second-level bullet beneath the bullet for its name, because a dog’s breed is not dependent/derived data from its name. Similarly, description is not dependent/derived from label, and so should not be nested in DOM. * fix: Reduce styled-components class permutations. https://www.joshwcomeau.com/css/styled-components/ * feat(Overlay): slide away from anchor based on position (#1322) * feat(Overlay): slide away from anchor based on position * fix: handle position changes when re-opening AnchoredOverlay * refactor: use js animation for slide to fix safari * fix: Tests were failing with Axe violations - https://dequeuniversity.com/rules/axe/4.1/aria-dialog-name - https://dequeuniversity.com/rules/axe/4.2/presentation-role-conflict - https://www.w3.org/TR/wai-aria-practices-1.1/examples/menu-button/menu-button-links.html First, 'Overlay's aren’t 'listbox'es, because (when used in 'DropdownMenu' or 'ActionMenu') they contain 'menuitem's, 'menuitemradio's, or 'menuitemcheckbox'es. Second, 'Overlay's aren’t 'dialog's, because (as demonstrated in the WAI ARIA practices page linked above), 'menu's need not be contained in a 'dialog', and also (as noted in the 'aria-dialog-name' link above), 'dialog's must have an 'aria-label', 'aria-labelledby', or 'title', but neither 'DropdownMenu' nor 'ActionMenu' have any kind of header element that could be used for this. Third, if 'Overlay's are 'none', they can’t be focusable (as noted in the 'presentation-role-conflict' link above), but one of our hooks (maybe 'FocusTrap', maybe 'FocusZone') was setting 'tabIndex' to '0' (in the test component), because it did not contain a focusable child. This PR adds a focusable button child so the 'none' 'Overlay' container won’t receive 'tabIndex' '0'. * fix: Resolve lint errors Co-authored-by: Clay Miller <[email protected]> * Generate prop documentation (#1323) * Add new filesystem source * Add component metadata type * Create Props component * Update props table * Handle empty and error states * Add required label * Update required prop styles * Clean up code comments * Remove filesystem plugin * Remove extra markdown file * Add component comment Co-authored-by: Clay Miller <[email protected]> * Improve treeshaking by setting package.json sideEffects (#1332) * fix: mark sideEffects free * fix: update sideEffects delcaration in package.json to improve treeshaking * fix: update sideEffects delcaration in package.json to improve treeshaking * fix: BaseStyles doesnt use sideeffects * chore: add changeset * Update Box documentation * Update BoxProps * Update Box docs * Update Box props * Remove AvatarStack story * Update .changeset/green-worms-nail.md Co-authored-by: Cole Bemis <[email protected]> Co-authored-by: Clay Miller <[email protected]> Co-authored-by: Dusty Greif <[email protected]> Co-authored-by: Matthew Costabile <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This introduces some additional refinements to
active-descendantand its use inActionList.mousemoveevent over any "focusable" item within the container. Practically speaking, this means mousing over items in anActionListwill cause the active descendant to move with the mouse. This is more in-line with MacOS system menus, which move focus with mouse and/or keyboarddata-is-active-descendant="activated-directly"ordata-is-active-descendant="activated-indirectly". The presence ofdata-is-active-descendanttells you that the item is active, andactivated-*gives insight into how it was activated. The only way to activate "directly" right now is to use the up/down arrows or directly focus an item (by clicking on it). All other interactions (mouseover, adding/removing elements, initial zone creation) are considered actions that moved active descendant "indirectly". Using these additional details, we can now renderActionListitems differently depending on how users interacted with the list.data-has-active-descendantto the container. This is helpful because thearia-activedescendantattribute is put on the control element.data-has-active-descendantand:focus-within, we are able to detect at the list level if there is an active or focused item within the list. If so, we want to ignore:hoverstyles to avoid confusion about which element will receive events if you pressEnter.SelectPanelto have a more proper active descendant setup. TheActionListis now thecontainer, and the input used for controlling the list is outside of the container. This required the addition offorwaredReffor theListcomponent.Closes https://github.com/github/primer/issues/199
Screenshots
SelectPanel (Active Descendant)
ActionMenu (Focus)
Merge checklist