-
Notifications
You must be signed in to change notification settings - Fork 640
Add gap prop to ActionBar for customizable spacing between items #7000
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
Conversation
🦋 Changeset detectedLatest commit: eb7541e The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
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.
Pull Request Overview
This PR adds a new gap prop to the ActionBar component to allow customizable spacing between items, with a default of 8 pixels. The implementation includes proper prop validation to cap the gap at 8 pixels maximum and 0 pixels minimum to maintain design consistency.
- Adds
gapprop to ActionBar with documentation and validation - Updates internal calculation functions to use the customizable gap value
- Provides Storybook examples demonstrating different gap values
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/react/src/ActionBar/ActionBar.tsx | Adds gap prop with validation logic and updates all spacing calculations to use the custom gap |
| packages/react/src/ActionBar/ActionBar.stories.tsx | Adds Storybook controls and examples showcasing different gap values |
|
Uh oh! @pksjce, at least one image you shared is missing helpful alt text. Check your pull request body to fix the following violations:
Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.
|
|
👋 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! |
Add gap prop to ActionBar for customizable spacing between items
| const {size = 'medium', children, 'aria-label': ariaLabel, flush = false, className} = props | ||
| const {size = 'medium', children, 'aria-label': ariaLabel, flush = false, className, gap} = props | ||
|
|
||
| const itemGap = Math.min(Math.max(gap ?? ACTIONBAR_ITEM_GAP, 0), ACTIONBAR_ITEM_GAP) |
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.
This keeps it between 0 and 8
|
|
||
| // GapExamples story moved to examples (Next.js example page) to reduce Storybook surface area. |
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.
is this accurate?
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.
ugh no!
| </ActionBar> | ||
| ) | ||
|
|
||
| export const GapScale = () => ( |
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.
should we add this to VRT?
Closes https://github.com/github/primer/issues/5980
Changelog
The gap between the icons in ActionBar was always 8. Now we can customise
New
Rollout strategy
Testing & Reviewing
Merge checklist