UnderlineNav: Show item icons above xsmall by default - #8260
Conversation
🦋 Changeset detectedLatest commit: 33913e2 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 |
liuliu-dev
added
the
Canary Release
|
github-actions
Bot
added
the
integration-tests: recommended
There was a problem hiding this comment.
Pull request overview
This PR updates UnderlineNav’s default icon-hiding behavior so item icons remain visible in most layouts by default, while preserving consumer configurability via hideIconsBreakpoint (including null to always show icons).
Changes:
- Changed
hideIconsBreakpointdefault fromxxlargetoxsmalland updated the prop’s JSDoc accordingly. - Added unit coverage for the default behavior, a custom breakpoint, and the
null“always show” configuration. - Updated the overflow Storybook story to explicitly set
hideIconsBreakpoint="medium"and adjusted an e2e interaction to be more deterministic before taking a snapshot.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/UnderlineNav/UnderlineNav.tsx | Switches the default hideIconsBreakpoint to xsmall and updates the prop documentation. |
| packages/react/src/UnderlineNav/UnderlineNav.test.tsx | Adds tests validating the new default, a custom breakpoint value, and null behavior. |
| packages/react/src/UnderlineNav/UnderlineNav.features.stories.tsx | Pins the overflow story to hideIconsBreakpoint="medium" to keep the example’s behavior intentional after the default change. |
| e2e/components/UnderlineNav.test.ts | Improves interaction stability by waiting for the menu item to disappear and the overflow button label to update before snapshotting. |
| .changeset/underline-nav-icon-breakpoint.md | Adds a patch changeset describing the consumer-visible default behavior change. |
Review details
- Files reviewed: 5/122 changed files
- Comments generated: 0
- Review effort level: Lite
| --- | ||
| '@primer/react': patch | ||
| --- | ||
|
|
||
| UnderlineNav: Keep item icons visible by default except below the xsmall breakpoint No newline at end of file |
There was a problem hiding this comment.
when was the original default added? depending on the timeline this has been around, might be worth considering whether this is a major change👀
patch: we accidentally added the wrong default value in a recent release and are correcting it
major: we're changing the behavior of this component by changing this default
There was a problem hiding this comment.
double checking that this diff is correct ? seems some shading around the "more" button is gone
There was a problem hiding this comment.
good catch! I reverted this snapshot change.
liuliu-dev
added
the
update snapshots
github-actions
Bot
removed
the
update snapshots
….com/primer/react into liuliu/underline-nav-icon-breakpoint
liuliu-dev
added
the
update snapshots
github-actions
Bot
removed
the
update snapshots
|
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. |
Closes github/primer#6929
UnderlineNavpreviously defaultedhideIconsBreakpointtoxxlarge, hiding item icons in containers narrower than 87.5rem.This changes the default to
xsmall, keeping icons visible in typical layouts while still allowing consumers to configure a larger breakpoint or passnullto always show icons.Changelog
Changed
hideIconsBreakpointfromxxlargetoxsmall.medium.Rollout strategy
Testing & Reviewing