feat(chip-group): Add Chip Group component #6075
Merged
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
github-actions
bot
added
the
enhancement
alisonailea
reviewed
Dec 16, 2022
alisonailea
reviewed
Dec 16, 2022
This was referenced Dec 16, 2022
macandcheese
added a commit
that referenced
this pull request
Dec 17, 2022
…ng (#6086) **Related Issue:** #6081 [#6803](#6083) ## Summary As suggested by @alisonailea [in this comment](#6075 (comment)), extracts some common keyboard navigation focus helpers into a shared utility. Along the way discovered a few things that are fixed alongside the refactors: Accordion: - Cleanup: The keyboard expected navigation changed at some point, which I confirmed with @geospatialem, but there was a lot of related stale code that is removed. Dropdown: - Fix: Previously, tab / shift tab would skip elements resulting in navigating to every other item. - Fix: Previously, there was inconsistent close behavior when using tab / shift tab at the first / last item. - Fix: Previously, when `accordion-item` had a populated `href` property, there was a "double focus" issue. - Cleanup: Uses the new utility to handle keyboard navigation / focus Stepper / Tabs: - Cleanup: Uses the new utility to handle keyboard navigation / focus All existing tests are passing, but we may want some extra passes of manual testing, especially in the dropdown case, since there were so many weird bugs previously.
macandcheese
added
the
pr ready for visual snapshots
macandcheese
added
pr ready for visual snapshots
macandcheese
added
pr ready for visual snapshots
macandcheese
added
pr ready for visual snapshots
macandcheese
removed
the
pr ready for visual snapshots
macandcheese
added
the
pr ready for visual snapshots
macandcheese
added
pr ready for visual snapshots
macandcheese
added
pr ready for visual snapshots
macandcheese
added
pr ready for visual snapshots
github-actions
bot
added this to the 2023 April Priorities - No breaking changes milestone
6 tasks
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.
Related Issue: #1933
Opening as a draftReady for review :)Summary
This still needs work but want to get a branch up so folks can preview and provide feedback.
This PR adds the proposed
chip-groupcomponent and some associated changes tochip.chip-groupcan be used for spacing and keyboard navigation of selectable or non-selectablechipcomponents.chip-groupdefaults toselection-mode:none, but supportssingle,single-persist, andmultipleselection modes.To do / looking for feedback:
Currently, users need to add
selectableproperty to childchipcomponents to enable the selection capability. This could potentially be managed by the parentchip-group: conditionally applied via an internal property based onselection-mode, but that creates a weird situation whereselectableis not documented but theselectedproperty is public, since that will always need to be available to a user. Open to thoughts there.Currently, once dismissed, the previous chip is focused. If the first chip is dismissed, focus is moved to the "next first chip" as it takes the place - does this make sense?
Add more tests as needed.
Audit of keyboard navigation.
Need an a11y audit from @geospatialem :)
Style cleanup / design feedback from @SkyeSeitz @ashetland :)
General pattern feedback :)
Screen.Recording.2022-12-16.at.9.18.05.AM.mov