Fix and refactor UnderlineNav to resolve CLS issues and improve performance/code quality#7506
Fix and refactor UnderlineNav to resolve CLS issues and improve performance/code quality#7506iansan5653 wants to merge 38 commits intomainfrom
UnderlineNav to resolve CLS issues and improve performance/code quality#7506Conversation
…e.module.css Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…e.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ate to detect overflow
…er themselves for the menu
|
|
👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the |
|
BTW, I think the flex inspector in Chrome's devtools does a really nice job of showing what's going on here / where the tabs are going: Screen.Recording.2026-02-06.at.5.00.33.PM.mov |
a787dd6 to
184ea86
Compare
|
@copilot Run and update VRT snapshots |
|
@iansan5653 I've opened a new pull request, #7580, to work on those changes. Once the pull request is ready, I'll request review from you. |
| // expect(await page.screenshot()).toMatchSnapshot() | ||
|
|
||
| await page.setViewportSize({width: viewports['primer.breakpoint.sm'], height: 768}) | ||
| await page.locator('button', {hasText: 'More Repository Items'}).waitFor() |
There was a problem hiding this comment.
I've updated the label of the overflow button to "More items", aligning with this issue for ActionBar: #7437
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
|
🤖 Lint and formatting issues have been automatically fixed and committed to this PR. |
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
This refact/bugfix PR reworks
UnderlineNav's internal logic to rely mostly on CSS for calculating overflow, eliminating layout shift issues. In addition, it removes the use of theReact.ChildrenAPI in favor of context-based solutions, and replaces the custom overflow menu with anActionMenu.See https://github.com/github/primer/issues/6291#issuecomment-3861950186 for context.
The CSS overflow logic works like this:
overflow: hidden, creating the illusion that they disappearedscroll-statecontainer queries to detect overflow using pure CSSIntersectionObserverto update itself with the parent when it appears/disappears. They determine whether they are overflowed by checkingref.current.offsetHeight(which will be greater than zero for an item that has wrapped). If they are overflowed, they setaria-hidden/tabIndex="-1"and notify the parent via the registry.The benefits are pretty significant:
ChildrenAPIs entirelyChildrenAPIs, we no longer need direct children, so consumers can wrap their underline items in fragments and even wrapper componentsHowever, there are some caveats / user-facing changes:
Changelog
New
Changed
Removed
Rollout strategy
Testing & Reviewing
Merge checklist