-
Notifications
You must be signed in to change notification settings - Fork 645
feat(LabelGroup): render as list by default #5156
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
feat(LabelGroup): render as list by default #5156
Conversation
🦋 Changeset detectedLatest commit: c9675ef 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 |
|
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
size-limit report 📦
|
…5-prclabelgroup-component-lacks-semantic-structure
…acks-semantic-structure
|
🟢 golden-jobs completed with status |
…acks-semantic-structure
|
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/348056 |
camertron
left a comment
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.
🎉
| <StyledLabelGroupContainer data-overflow="inline" data-list={isList || undefined} sx={sxProp} as={as}> | ||
| {isList | ||
| ? React.Children.map(children, child => { | ||
| return <li>{child}</li> |
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.
Do these need a key prop?
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.
Yes! done , Thanks!!
…5-prclabelgroup-component-lacks-semantic-structure
Closes https://github.com/github/primer/issues/3425
Renders the
LabelGroupas aulwithlis by default, with the option to useasprop to customize the element type of the rendered container. In the case other thanulorol, the children will be rendered asspan.Changelog
New
asprop inLabelGroupto customize container element (provides backwards compatibility)ulinLabelGroupdata-listattribute inStyledLabelGroupContainer(used to apply styles conditionally)LabelGroupchildren now wrapped by<li></li>component when rendered as listLabelGrouptests to account for new functionality and use casesChanged
LabelGrouprenders as aulby default, with option to override default implementation viaasprop.Rollout strategy
Testing & Reviewing
Check DOM in deployed story
, verify it renders as a
ulwithlis. No visual regressions should be present.Merge checklist