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 98 commits into
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
🦋 Changeset detectedLatest commit: 8a780ac 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-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
…-nav-full-css-spike
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
1 similar comment
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
…ile to resolve react-refresh error
…mer/react into underline-nav-full-css-spike
…-nav-full-css-spike
|
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. |
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
1 similar comment
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
|
Testing in preview showed a few small issues:
|
|
🤖 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:
Screen.Recording.2026-06-19.at.4.28.38.PM.mov
Changelog
New
Changed
Removed
Rollout strategy
Testing & Reviewing
Merge checklist