-
Notifications
You must be signed in to change notification settings - Fork 598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ActionList Fixes #1215
ActionList Fixes #1215
Conversation
🦋 Changeset detectedLatest commit: 2d604c6 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/AwfrL2RE61W7UMpqyhpBnACmCd7L |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
.changeset/blue-items-repeat.md
Outdated
Hide divider before `ActionList.Group`s with filled header | ||
Align `Item` description to when rendered in-line | ||
Add `selectionVariant: 'multiple'` for `Item`s. These will use a checkbox input instead of a checkmark icon for selected state | ||
Allow `focusZoneSettings` to be passed into `AnchoredOverlay` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's not too much trouble, it might be nice to put all these distinct changes in their own changeset.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, @dgreif! Thanks for cleaning this up ❤️
Just left a couple non-blocking comments.
… 'Dialog', to avoid clipping focus ring outlines (particularly in Safari). Because this would collapse margins, add 'position: absolute' to 'ErsatzOverlay' to avoid collapsing 'List'’s top and bottom margins
Addressing a number of issues found when integrating
ActionList
intoSelectPanel
inline
description is not inlineWrap item in a(Using<label>
?aria-label={text}
to avoid branching DOM and for succinct announcements)Update docs with new(Prop is commented in theselectionVariant
optionItemProps
interface definition, butActionList.Item
does not have a corresponding.mdx
file)Screenshots
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.