Skip to content

WIP: Fix UnderlineNav CLS by using wrapping to overflow items onto the next (hidden) line #7505

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.

Sign up for GitHub

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

Draft
iansan5653 wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

WIP: Fix UnderlineNav CLS by using wrapping to overflow items onto the next (hidden) line #7505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7b8e794
Add overflow: hidden logic
TylerJDev Feb 6, 2026
a0e4f42
Add changeset
TylerJDev Feb 6, 2026
604c035
Run format
TylerJDev Feb 6, 2026
7fc6739
Update packages/react/src/internal/components/UnderlineTabbedInterfac…
TylerJDev Feb 6, 2026
6921a86
Update packages/react/src/internal/components/UnderlineTabbedInterfac…
TylerJDev Feb 6, 2026
1bc03b6
Update packages/react/src/UnderlineNav/UnderlineNav.tsx
TylerJDev Feb 6, 2026
cc8b23b
WIP: wrap items out of the way during initial render, using scroll-st…
iansan5653 Feb 6, 2026
1835cfc
Remove item opacity
iansan5653 Feb 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/clever-geese-cover.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

UnderlineNav: Adds `overflow: hidden` when calculating items to prevent CLS
9 changes: 9 additions & 0 deletions packages/react/src/UnderlineNav/UnderlineNav.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@
justify-content: space-between;
}

.MoreButtonContainer {
display: none;
align-items: center;

@container underline-wrapper scroll-state(scrollable: block) {
display: flex;
}
}

/* More button styles migrated from styles.ts (was moreBtnStyles) */
.MoreButton {
margin: 0; /* reset Safari extra margin */
Expand Down
214 changes: 118 additions & 96 deletions packages/react/src/UnderlineNav/UnderlineNav.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export type UnderlineNavProps = {
// However, we need to calculate number of possible items when the more button present as well. So using the width of the more button as a constant.
export const MORE_BTN_WIDTH = 86
// The height is needed to make sure we don't have a layout shift when the more button is the only item in the nav.
const MORE_BTN_HEIGHT = 45
const MORE_BTN_HEIGHT = 32

const overflowEffect = (
navWidth: number,
Expand Down Expand Up @@ -166,6 +166,8 @@ export const UnderlineNav = forwardRef(
const [iconsVisible, setIconsVisible] = useState<boolean>(true)
const [childWidthArray, setChildWidthArray] = useState<ChildWidthArray>([])
const [noIconChildWidthArray, setNoIconChildWidthArray] = useState<ChildWidthArray>([])
// Track whether the initial overflow calculation is complete to prevent CLS
const [isReady, setIsReady] = useState(false)

const validChildren = getValidChildren(children)

Expand Down Expand Up @@ -245,10 +247,19 @@ export const UnderlineNav = forwardRef(
return breakpoint
}

const updateListAndMenu = useCallback((props: ResponsiveProps, displayIcons: boolean) => {
setResponsiveProps(props)
setIconsVisible(displayIcons)
}, [])
const updateListAndMenu = useCallback(
(props: ResponsiveProps, displayIcons: boolean) => {
setResponsiveProps(props)
setIconsVisible(displayIcons)

// Only mark as ready once widths have been measured for all valid children
const widths = displayIcons ? childWidthArray : noIconChildWidthArray
if (!isReady && widths.length > 0 && widths.length >= validChildren.length) {
setIsReady(true)
}
},
[childWidthArray, noIconChildWidthArray, isReady, validChildren.length],
)
const setChildrenWidth = useCallback((size: ChildSize) => {
setChildWidthArray(arr => {
const newArr = [...arr, size]
Expand Down Expand Up @@ -330,100 +341,111 @@ export const UnderlineNav = forwardRef(
}}
>
{ariaLabel && <VisuallyHidden as="h2">{`${ariaLabel} navigation`}</VisuallyHidden>}
<UnderlineWrapper as={as} aria-label={ariaLabel} className={className} ref={navRef} data-variant={variant}>
<UnderlineWrapper
as={as}
aria-label={ariaLabel}
className={className}
ref={navRef}
data-variant={variant}
ready={isReady}
>
<UnderlineItemList ref={listRef} role="list">
{listItems}
{menuItems.length > 0 && (
<li ref={moreMenuRef} style={{display: 'flex', alignItems: 'center', height: `${MORE_BTN_HEIGHT}px`}}>
{!onlyMenuVisible && <div style={dividerStyles}></div>}
<Button
ref={moreMenuBtnRef}
className={classes.MoreButton}
aria-controls={disclosureWidgetId}
aria-expanded={isWidgetOpen}
onClick={onAnchorClick}
trailingAction={TriangleDownIcon}
>
<span>
{onlyMenuVisible ? (
<>
<VisuallyHidden as="span">{`${ariaLabel}`}&nbsp;</VisuallyHidden>Menu
</>
) : (
<>
More<VisuallyHidden as="span">&nbsp;{`${ariaLabel} items`}</VisuallyHidden>
</>
)}
</span>
</Button>
<ActionList
selectionVariant="single"
ref={containerRef}
id={disclosureWidgetId}
style={{
...(listRef.current?.clientWidth && listRef.current.clientWidth >= baseMenuMinWidth
? baseMenuInlineStyles
: menuInlineStyles),
display: isWidgetOpen ? 'block' : 'none',
}}
>
{menuItems.map((menuItem, index) => {
const {
children: menuItemChildren,
counter,
'aria-current': ariaCurrent,
onSelect,
...menuItemProps
} = menuItem.props

// This logic is used to pop the selected item out of the menu and into the list when the navigation is control externally
if (Boolean(ariaCurrent) && ariaCurrent !== 'false') {
const event = new MouseEvent('click')
!onlyMenuVisible &&
swapMenuItemWithListItem(
menuItem,
index,
// @ts-ignore - not a big deal because it is internally creating an event but ask help
event as React.MouseEvent<HTMLAnchorElement>,
updateListAndMenu,
)
}

return (
<ActionList.LinkItem
key={menuItemChildren}
style={menuItemStyles}
onClick={(
event: React.MouseEvent<HTMLAnchorElement> | React.KeyboardEvent<HTMLAnchorElement>,
) => {
// When there are no items in the list, do not run the swap function as we want to keep everything in the menu.
!onlyMenuVisible && swapMenuItemWithListItem(menuItem, index, event, updateListAndMenu)
closeOverlay()
focusOnMoreMenuBtn()
// fire onSelect event that comes from the UnderlineNav.Item (if it is defined)
typeof onSelect === 'function' && onSelect(event)
}}
{...menuItemProps}
>
<span className={classes.MenuItemContent}>
{menuItemChildren}
{loadingCounters ? (
<LoadingCounter />
) : (
counter !== undefined && (
<span data-component="counter">
<CounterLabel>{counter}</CounterLabel>
</span>
)
)}
</span>
</ActionList.LinkItem>
)
})}
</ActionList>
</li>
)}
</UnderlineItemList>
<div
ref={moreMenuRef}
style={{
display: menuItems.length > 0 ? 'flex' : undefined,
alignItems: 'center',
height: `${MORE_BTN_HEIGHT}px`,
}}
className={classes.MoreButtonContainer}
>
{!onlyMenuVisible && <div style={dividerStyles}></div>}
<Button
ref={moreMenuBtnRef}
className={classes.MoreButton}
aria-controls={disclosureWidgetId}
aria-expanded={isWidgetOpen}
onClick={onAnchorClick}
trailingAction={TriangleDownIcon}
>
<span>
{onlyMenuVisible ? (
<>
<VisuallyHidden as="span">{`${ariaLabel}`}&nbsp;</VisuallyHidden>Menu
</>
) : (
<>
More<VisuallyHidden as="span">&nbsp;{`${ariaLabel} items`}</VisuallyHidden>
</>
)}
</span>
</Button>
<ActionList
selectionVariant="single"
ref={containerRef}
id={disclosureWidgetId}
style={{
...(listRef.current?.clientWidth && listRef.current.clientWidth >= baseMenuMinWidth
? baseMenuInlineStyles
: menuInlineStyles),
display: isWidgetOpen ? 'block' : 'none',
}}
>
{menuItems.map((menuItem, index) => {
const {
children: menuItemChildren,
counter,
'aria-current': ariaCurrent,
onSelect,
...menuItemProps
} = menuItem.props

// This logic is used to pop the selected item out of the menu and into the list when the navigation is control externally
if (Boolean(ariaCurrent) && ariaCurrent !== 'false') {
const event = new MouseEvent('click')
!onlyMenuVisible &&
swapMenuItemWithListItem(
menuItem,
index,
// @ts-ignore - not a big deal because it is internally creating an event but ask help
event as React.MouseEvent<HTMLAnchorElement>,
updateListAndMenu,
)
}

return (
<ActionList.LinkItem
key={menuItemChildren}
style={menuItemStyles}
onClick={(event: React.MouseEvent<HTMLAnchorElement> | React.KeyboardEvent<HTMLAnchorElement>) => {
// When there are no items in the list, do not run the swap function as we want to keep everything in the menu.
!onlyMenuVisible && swapMenuItemWithListItem(menuItem, index, event, updateListAndMenu)
closeOverlay()
focusOnMoreMenuBtn()
// fire onSelect event that comes from the UnderlineNav.Item (if it is defined)
typeof onSelect === 'function' && onSelect(event)
}}
{...menuItemProps}
>
<span className={classes.MenuItemContent}>
{menuItemChildren}
{loadingCounters ? (
<LoadingCounter />
) : (
counter !== undefined && (
<span data-component="counter">
<CounterLabel>{counter}</CounterLabel>
</span>
)
)}
</span>
</ActionList.LinkItem>
)
})}
</ActionList>
</div>
</UnderlineWrapper>
</UnderlineNavContext.Provider>
)
Expand Down
27 changes: 24 additions & 3 deletions packages/react/src/internal/components/UnderlineTabbedInterface.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,33 @@
display: flex;
/* stylelint-disable-next-line primer/spacing */
padding-inline: var(--stack-padding-normal);
justify-content: flex-start;
align-items: center;
padding-top: var(--base-size-8);
justify-content: space-between;
align-items: flex-start;

/* make space for the underline */
min-height: var(--control-xlarge-size, 48px);
height: var(--control-xlarge-size, 48px);

/* using a box-shadow instead of a border to accommodate 'overflow-y: hidden' on UnderlinePanels */
/* stylelint-disable-next-line primer/box-shadow */
box-shadow: inset 0 -1px var(--borderColor-muted);

/* Hide overflow until calculation is complete to prevent CLS */
overflow: visible;

&[data-ready='false'] {
overflow: hidden;
container: underline-wrapper / scroll-state;

.UnderlineItemList {
flex-wrap: wrap;
}
}

&[data-ready='true'] {
overflow: visible;
}

&[data-variant='flush'] {
/* stylelint-disable-next-line primer/spacing */
padding-inline: unset;
Expand Down Expand Up @@ -91,6 +108,10 @@
color: var(--fgColor-muted);
align-items: center;
margin-inline-end: var(--base-size-8);

@container underline-wrapper scroll-state(scrollable: block) {
display: none;
}
}

.UnderlineItem [data-component='counter'] {
Expand Down
6 changes: 5 additions & 1 deletion packages/react/src/internal/components/UnderlineTabbedInterface.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,18 @@ type UnderlineWrapperProps<As extends React.ElementType> = {
as?: As
className?: string
ref?: React.Ref<HTMLElement>
/** Indicates whether the overflow calculation is complete. When false, overflow is hidden to prevent CLS. */
ready?: boolean
}

export const UnderlineWrapper = forwardRef((props, ref) => {
const {children, className, as: Component = 'div', ...rest} = props
const {children, className, as: Component = 'div', ready, ...rest} = props

return (
<Component
className={clsx(classes.UnderlineWrapper, className)}
ref={ref as ForwardedRef<HTMLDivElement>}
data-ready={ready === undefined ? undefined : ready ? 'true' : 'false'}
{...rest}
>
{children}
Expand Down
Loading