WIP: Fix UnderlineNav CLS and improve performance/code quality by rebuilding the whole overflow logic to use mostly CSS#7506
Conversation
…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

This proof of concept goes a step beyond my previous spike (#7505) by ripping out all of the complex layout calculation logic and replacing it with CSS. See https://github.com/github/primer/issues/6291#issuecomment-3861950186 for context.
It works like this:
overflow: hidden, creating the illusion that they disappearedscroll-statecontainer queries to detect overflow using pure CSScontainerWidth(tracked viaResizeObserver) and register/unregister functions through contextcontainerWidthorpropschange. In this effect, 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/tabIndexand notify the parent via the registry.The benefits are pretty significant:
ChildrenAPIs entirelyChildrenAPIs, we no longer need direct children, so consumers can wrap their items in fragments and even subcomponentsHowever, there are caveats:
overflow: hiddenfor the component lifecycle, so tabs cannot display popovers and tooltips without using portals. In my other PR, we are able to removeoverflow: hiddenafter the initial calcs. However, this is relatively easy to work around: use the native CSSpopoverfor tooltips (as we already are inTooltip) and usePortal/Overlayfor popovers rather than rendering DOM into the menu (which is an accessibility violation anyway)overflow: hidden? I'm not sure but it seems fixable. Maybe we can move toAnchoredOverlayhereWrapped items still take up horizontal space, so if you have a really long wrapped item it can prevent other items from wrapping. I think maybe we can solve this with some flex-shrink on the items?Found a CSS-based solution for thiscontainerWidthis smaller than that item. Although this seems like a rare edge case to me and I wonder if only showing "More" ever actually makes sense or would it just be confusing? Maybe the last tab should just get an ellipsis truncation instead of moving to the menu?scroll-statefunction. The React 19 build does. Do we still need this build, given that dotcom is fully migrated to React 19 now? Not sure how else we could work around this 😕Closes #
Changelog
New
Changed
Removed
Rollout strategy
Testing & Reviewing
Merge checklist