ActionList: add gap between items behind feature flag#7976
Conversation
🦋 Changeset detectedLatest commit: ccc2b4f 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 |
Co-authored-by: jonrohan <54012+jonrohan@users.noreply.github.com>
|
github-actions
Bot
added
the
integration-tests: recommended
There was a problem hiding this comment.
Pull request overview
Adds an opt-in (feature-flagged) visual gap between adjacent ActionList items to improve readability without changing default layouts.
Changes:
- Introduces
primer_react_action_list_item_gap(defaultfalse) in the default feature flag set. - When enabled,
ActionListsetsdata-item-gapon the list root and CSS applies a 2pxmargin-block-startbetween adjacent.ActionListItemsiblings. - Adds unit tests and a Storybook “feature” story demonstrating the flag-enabled appearance, plus a patch changeset.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/FeatureFlags/DefaultFeatureFlags.ts | Registers the new feature flag with a default of false. |
| packages/react/src/ActionList/List.tsx | Reads the flag via useFeatureFlag and conditionally adds data-item-gap to the ActionList root. |
| packages/react/src/ActionList/ActionList.test.tsx | Verifies the data-item-gap attribute is absent by default and present when the flag is enabled. |
| packages/react/src/ActionList/ActionList.module.css | Adds a [data-item-gap]-scoped rule that applies a 2px gap between adjacent items. |
| packages/react/src/ActionList/ActionList.features.stories.tsx | Adds a Storybook feature story rendering ActionList with the flag enabled. |
| .changeset/action-list-item-gap.md | Patch changeset describing the feature-flagged spacing adjustment. |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 1
| it('does not set data-item-gap by default', () => { | ||
| const {container} = HTMLRender( |
|
Integration test results from github/github-ui PR:
VRT check ensures that when visual differences are detected, the PR cannot proceed until someone acknowledges the changes by adding the "visual difference acknowledged" label. Need help? If you believe this failure is unrelated to your changes, please reach out to the Primer team for assistance. |
francinelucca
left a comment
francinelucca
left a comment
There was a problem hiding this comment.
approving as not to block this, but wondering if we might want to scope this just down to NavList or something 🤔. IDK that it's necessary for things like SelectPanel or plain ActionList, happy to defer to design though!
Fixes https://github.com/github/primer/issues/6612
ActionListitems render with no vertical separation, so adjacent hover/active backgrounds touch and look cramped. This adds a small gap between sibling items, gated behind a feature flag to avoid disturbing existing layouts.Behavior is opt-in via
primer_react_action_list_item_gap. When enabled, the list root gets adata-item-gapattribute and CSS appliesmargin-block-start: var(--base-size-2)(2px) between adjacent items.Changelog
New
primer_react_action_list_item_gapfeature flag (defaultfalse).data-item-gapattribute on theActionListroot when the flag is enabled..ActionListItemsiblings under[data-item-gap].Changed
Removed
Rollout strategy
Testing & Reviewing
Toggle the
primer_react_action_list_item_gapflag in the feature story (or via the Storybook toolbar) and confirm items gain 2px of vertical separation; with the flag off, spacing is unchanged. Unit tests cover both default-off and flag-on states.Open question carried over from the issue: the gap is set to the requested 2px minimum (
--base-size-2); design still needs to confirm whether 4px is preferred — only the single token in the CSS rule changes if so.Merge checklist