From 780b0a32cb08a314e972749ecb11d25c08cbed23 Mon Sep 17 00:00:00 2001 From: Matthew Costabile <8616962+mattcosta7@users.noreply.github.com> Date: Wed, 4 Nov 2026 22:08:04 -0500 Subject: [PATCH 1/3] perf(ActionList, TreeView): avoid whole-subtree style invalidation on hover, focus and attribute changes ActionList: key the inactive/disabled universal descendant color rules on private classes set on the
  • instead of :hover and data/aria attributes. Blink builds whole-subtree invalidation sets per feature page-wide, so the old selectors restyled the entire subtree of any element whose hover state or aria-disabled/data-inactive attribute changed. Computed colors are unchanged. TreeView: replace the inherited --tree-line-color toggle on the root with a class-targeted descendant rule so pointer enter/leave and focus in/out only restyle the level lines. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...tionlist-inactive-disabled-invalidation.md | 5 +++ .../treeview-level-line-invalidation.md | 5 +++ .../src/ActionList/ActionList.module.css | 33 ++++++++++---- packages/react/src/ActionList/Item.test.tsx | 43 +++++++++++++++++++ packages/react/src/ActionList/Item.tsx | 13 +++++- .../react/src/TreeView/TreeView.module.css | 29 +++++++------ 6 files changed, 105 insertions(+), 23 deletions(-) create mode 100644 .changeset/actionlist-inactive-disabled-invalidation.md create mode 100644 .changeset/treeview-level-line-invalidation.md diff --git a/.changeset/actionlist-inactive-disabled-invalidation.md b/.changeset/actionlist-inactive-disabled-invalidation.md new file mode 100644 index 00000000000..adc60584833 --- /dev/null +++ b/.changeset/actionlist-inactive-disabled-invalidation.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +ActionList: Reduce style recalculation when unrelated elements on the page change hover state or attributes diff --git a/.changeset/treeview-level-line-invalidation.md b/.changeset/treeview-level-line-invalidation.md new file mode 100644 index 00000000000..24d27bace14 --- /dev/null +++ b/.changeset/treeview-level-line-invalidation.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +TreeView: Reduce style recalculation when the pointer or focus enters or leaves the tree diff --git a/packages/react/src/ActionList/ActionList.module.css b/packages/react/src/ActionList/ActionList.module.css index 7a4a9065c0a..14f8028029e 100644 --- a/packages/react/src/ActionList/ActionList.module.css +++ b/packages/react/src/ActionList/ActionList.module.css @@ -267,17 +267,17 @@ /* inactive */ &:where([data-inactive='true']) { - /* ignore tooltip */ - & * :not([popover], .InactiveWarning) { - color: var(--fgColor-muted); - } + /* text color is set by `.InactiveColors` below */ @media (hover: hover) { &:hover { cursor: not-allowed; background-color: transparent; - & * :not([popover], .InactiveWarning) { + /* outrank the danger variant's hover colors */ + & .LeadingAction, + & .LeadingVisual, + & .ItemLabel { color: var(--fgColor-muted); } } @@ -316,6 +316,25 @@ } } + /* + * Inactive and disabled items mute every descendant, including nested components that set + * their own color. `Item.tsx` sets `.InactiveColors` and `.DisabledColors` on the `
  • `. + * + * Key these universal descendant selectors on those classes, never on `:hover` or an + * attribute. Blink builds invalidation sets per feature for the whole page, so `:hover *` or + * `[aria-disabled] *` would restyle the entire subtree of any element, in any component, + * whose hover state or attribute changes. + */ + + /* ignore tooltip */ + &.InactiveColors * :not([popover], .InactiveWarning) { + color: var(--fgColor-muted); + } + + &.DisabledColors .ActionListContent * { + color: var(--control-fgColor-disabled); + } + /* if item has subitem, move hover styles to ActionListContent */ &[data-has-subitem='true'] { /* first child */ @@ -343,9 +362,7 @@ &[aria-disabled='true'], &[data-is-disabled] { - & .ActionListContent * { - color: var(--control-fgColor-disabled); - } + /* text color is set by `.DisabledColors` above */ & .ActionListContent { @media (hover: hover) { diff --git a/packages/react/src/ActionList/Item.test.tsx b/packages/react/src/ActionList/Item.test.tsx index 0a34bacba76..b037b75e1a3 100644 --- a/packages/react/src/ActionList/Item.test.tsx +++ b/packages/react/src/ActionList/Item.test.tsx @@ -6,6 +6,7 @@ import {ActionList} from '.' import {AnchoredOverlay} from '../AnchoredOverlay' import {BookIcon} from '@primer/octicons-react' import {implementsClassName} from '../utils/testing' +import CounterLabel from '../CounterLabel' import classes from './ActionList.module.css' function SimpleActionList(): JSX.Element { @@ -601,4 +602,46 @@ describe('ActionList.Item', () => { expect(action).not.toHaveAttribute('data-has-label') }) }) + + describe('inactive and disabled text color', () => { + function renderItem(props: React.ComponentProps) { + const {container} = HTMLRender( + + + Label + + 3 + + + + Enabled + + 4 + + + , + ) + const [item, enabled] = container.querySelectorAll('[data-component="ActionList.Item"]') + const colorOf = (el: Element | null) => getComputedStyle(el!).color + return { + label: colorOf(item.querySelector(`.${classes.ItemLabel}`)), + nested: colorOf(item.querySelector('[data-component="CounterLabel"]')), + enabledNested: colorOf(enabled.querySelector('[data-component="CounterLabel"]')), + warning: item.querySelector(`.${classes.InactiveWarning}`), + } + } + + it('mutes nested components that set their own color in inactive items', () => { + const {label, nested, enabledNested, warning} = renderItem({inactiveText: 'Unavailable'}) + expect(nested).toBe(label) + expect(nested).not.toBe(enabledNested) + expect(getComputedStyle(warning!).color).not.toBe(label) + }) + + it('mutes nested components that set their own color in disabled items', () => { + const {label, nested, enabledNested} = renderItem({disabled: true}) + expect(nested).toBe(label) + expect(nested).not.toBe(enabledNested) + }) + }) }) diff --git a/packages/react/src/ActionList/Item.tsx b/packages/react/src/ActionList/Item.tsx index f811925983f..ab72fa5f0a8 100644 --- a/packages/react/src/ActionList/Item.tsx +++ b/packages/react/src/ActionList/Item.tsx @@ -318,6 +318,12 @@ const UnwrappedItem = ( // attributes so the CSS only kicks in when the action is actually in the DOM. const trailingActionRendered = !inactive && !loading && !menuContext && Boolean(slots.trailingAction) + // Mirrors the `
  • ` attributes below so CSS can key descendant colors on classes instead of + // attributes. See `.InactiveColors` in ActionList.module.css. + const liAriaDisabled = 'aria-disabled' in containerProps ? containerProps['aria-disabled'] : undefined + const disabledColors = disabled || liAriaDisabled === true || liAriaDisabled === 'true' + const inactiveColors = Boolean(inactiveText) && !disabled && !slots.subItem + return (
  • ( data-has-description={slots.description ? true : false} data-has-trailing-action={trailingActionRendered ? true : undefined} data-trailing-action-loading={trailingActionRendered && slots.trailingAction?.props.loading ? true : undefined} - className={clsx(classes.ActionListItem, className)} + className={clsx( + classes.ActionListItem, + inactiveColors && classes.InactiveColors, + disabledColors && classes.DisabledColors, + className, + )} > ` and inherited down. On coarse pointers it - * stays unset and falls back to `muted` (lines always visible). On hover-capable devices it - * is initialized to `transparent` on the root and flipped to `muted` while the tree is - * hovered or focused, so the browser only has to propagate a single inherited custom - * property instead of re-matching `.TreeViewItemLevelLine` descendant selectors on every - * hover/focus change inside large trees. - */ - /* stylelint-disable-next-line primer/colors -- private custom property, defaults to a Primer token */ - border-color: var(--tree-line-color, var(--borderColor-muted)); + /* On devices without hover, the nesting indicator lines are always visible. */ + border-color: var(--borderColor-muted); } + /* + * On devices with hover, the lines fade in while the tree is hovered or has focus. + * + * Keep this a class-targeted descendant rule. Toggling an inherited custom property on the + * root instead forces every element in the tree to restyle on each pointer enter/leave and + * focus in/out; this rule restyles only the `.TreeViewItemLevelLine` elements. + */ @media (hover: hover) { - --tree-line-color: transparent; + .TreeViewItemLevelLine { + border-color: transparent; + } - &:hover, - &:focus-within { - --tree-line-color: var(--borderColor-muted); + &:hover .TreeViewItemLevelLine, + &:focus-within .TreeViewItemLevelLine { + border-color: var(--borderColor-muted); } } From 203b2374431dda34be2cfbc9a595468c169bdcdd Mon Sep 17 00:00:00 2001 From: Matthew Costabile <8616962+mattcosta7@users.noreply.github.com> Date: Thu, 5 Nov 2026 08:35:37 -0500 Subject: [PATCH 2/3] fix(ActionList): keep inactive hover color on items that also set aria-disabled An item with inactiveText and an aria-disabled prop (without disabled) used the inactive color for every descendant while hovered. The class-keyed rules only muted the label and visuals, so nested components such as CounterLabel kept the disabled color. Drive the disabled color through a private custom property that the hovered inactive item overrides, so the hover change stays scoped to the item instead of a page-wide :hover descendant set. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/ActionList/ActionList.module.css | 13 ++++++++- packages/react/src/ActionList/Item.test.tsx | 27 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/packages/react/src/ActionList/ActionList.module.css b/packages/react/src/ActionList/ActionList.module.css index 14f8028029e..35a3058a256 100644 --- a/packages/react/src/ActionList/ActionList.module.css +++ b/packages/react/src/ActionList/ActionList.module.css @@ -280,6 +280,11 @@ & .ItemLabel { color: var(--fgColor-muted); } + + /* an `aria-disabled` prop without `disabled`: the inactive color wins while hovered */ + &.DisabledColors { + --disabledItem-fgColor: var(--fgColor-muted); + } } } @@ -332,7 +337,13 @@ } &.DisabledColors .ActionListContent * { - color: var(--control-fgColor-disabled); + /* stylelint-disable-next-line primer/colors -- private custom property, defaults to a Primer token */ + color: var(--disabledItem-fgColor, var(--control-fgColor-disabled)); + } + + /* the tooltip and warning keep the disabled color, even while hovered */ + &.DisabledColors .ActionListContent :is([popover], .InactiveWarning) { + --disabledItem-fgColor: var(--control-fgColor-disabled); } /* if item has subitem, move hover styles to ActionListContent */ diff --git a/packages/react/src/ActionList/Item.test.tsx b/packages/react/src/ActionList/Item.test.tsx index b037b75e1a3..078f9278b8b 100644 --- a/packages/react/src/ActionList/Item.test.tsx +++ b/packages/react/src/ActionList/Item.test.tsx @@ -1,6 +1,7 @@ import {describe, it, expect, vi} from 'vitest' import {render as HTMLRender, waitFor, fireEvent} from '@testing-library/react' import userEvent from '@testing-library/user-event' +import {userEvent as browserUserEvent} from 'vitest/browser' import React, {type JSX} from 'react' import {ActionList} from '.' import {AnchoredOverlay} from '../AnchoredOverlay' @@ -643,5 +644,31 @@ describe('ActionList.Item', () => { expect(nested).toBe(label) expect(nested).not.toBe(enabledNested) }) + + it('mutes every descendant while hovering an inactive item that also sets aria-disabled', async () => { + const {container} = HTMLRender( + + + + + + Label + + 3 + + + Inactive + , + ) + const [item, inactiveOnly] = container.querySelectorAll('[data-component="ActionList.Item"]') + const colorOf = (el: Element | null) => getComputedStyle(el!).color + const mutedColor = colorOf(inactiveOnly.querySelector(`.${classes.ItemLabel}`)) + + await browserUserEvent.hover(item.querySelector(`.${classes.ItemLabel}`)!) + + await expect.poll(() => colorOf(item.querySelector(`.${classes.ItemLabel}`))).toBe(mutedColor) + expect(colorOf(item.querySelector('[data-component="CounterLabel"]'))).toBe(mutedColor) + expect(colorOf(item.querySelector(`.${classes.ActionListSubContent}`))).toBe(mutedColor) + }) }) }) From dbe493f5efd3e4f7a5ad6a410beecce5eab1020b Mon Sep 17 00:00:00 2001 From: Matthew Costabile <8616962+mattcosta7@users.noreply.github.com> Date: Sun, 8 Nov 2026 00:02:40 -0500 Subject: [PATCH 3/3] TreeView: drive level-line focus state from data-focus-within WebKit clears and re-sets :focus-within up the ancestor chain on every focus move, so `:focus-within .TreeViewItemLevelLine` restyled every level line on each arrow key press (0.8ms -> 7.7ms per move on a 1,510-item tree vs main). useFocusWithinAttribute mirrors focus-within onto the root only when focus enters or leaves the tree. :hover stays in CSS. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../treeview-level-line-invalidation.md | 2 +- .../react/src/TreeView/TreeView.module.css | 6 +++- packages/react/src/TreeView/TreeView.test.tsx | 30 +++++++++++++++++ packages/react/src/TreeView/TreeView.tsx | 2 ++ .../src/TreeView/useFocusWithinAttribute.ts | 33 +++++++++++++++++++ 5 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 packages/react/src/TreeView/useFocusWithinAttribute.ts diff --git a/.changeset/treeview-level-line-invalidation.md b/.changeset/treeview-level-line-invalidation.md index 24d27bace14..9f0cfc7edef 100644 --- a/.changeset/treeview-level-line-invalidation.md +++ b/.changeset/treeview-level-line-invalidation.md @@ -2,4 +2,4 @@ '@primer/react': patch --- -TreeView: Reduce style recalculation when the pointer or focus enters or leaves the tree +TreeView: Reduce style recalculation when the pointer or focus enters or leaves the tree, and when focus moves between items in Safari diff --git a/packages/react/src/TreeView/TreeView.module.css b/packages/react/src/TreeView/TreeView.module.css index db51b2fc3a0..5361b62dd2a 100644 --- a/packages/react/src/TreeView/TreeView.module.css +++ b/packages/react/src/TreeView/TreeView.module.css @@ -224,6 +224,10 @@ * Keep this a class-targeted descendant rule. Toggling an inherited custom property on the * root instead forces every element in the tree to restyle on each pointer enter/leave and * focus in/out; this rule restyles only the `.TreeViewItemLevelLine` elements. + * + * Focus uses `data-focus-within` (set by `useFocusWithinAttribute`), not `:focus-within`: + * WebKit re-toggles `:focus-within` on every focus move inside the tree, which restyled every + * level line on each arrow key press. */ @media (hover: hover) { .TreeViewItemLevelLine { @@ -231,7 +235,7 @@ } &:hover .TreeViewItemLevelLine, - &:focus-within .TreeViewItemLevelLine { + &[data-focus-within] .TreeViewItemLevelLine { border-color: var(--borderColor-muted); } } diff --git a/packages/react/src/TreeView/TreeView.test.tsx b/packages/react/src/TreeView/TreeView.test.tsx index 1b703083c71..a775c5fe97f 100644 --- a/packages/react/src/TreeView/TreeView.test.tsx +++ b/packages/react/src/TreeView/TreeView.test.tsx @@ -581,6 +581,36 @@ describe('SubTree accessible name', () => { }) }) +describe('Focus within', () => { + it('sets data-focus-within on the root only while focus is inside the tree', async () => { + render( + <> + + + Item 1 + Item 2 + + + , + ) + const root = page.getByRole('tree').element() + + expect(root).not.toHaveAttribute('data-focus-within') + + act(() => page.getByRole('button', {name: 'Before'}).element().focus()) + await userEvent.keyboard('{Tab}') + expect(root).toHaveAttribute('data-focus-within') + + await userEvent.keyboard('{ArrowDown}') + await expect.element(page.getByRole('treeitem', {name: 'Item 2'})).toHaveFocus() + expect(root).toHaveAttribute('data-focus-within') + + await userEvent.keyboard('{Tab}') + await expect.element(page.getByRole('button', {name: 'After'})).toHaveFocus() + expect(root).not.toHaveAttribute('data-focus-within') + }) +}) + describe('Tab order', () => { const getTabStops = (container: Element) => Array.from(container.querySelectorAll('[tabindex="0"]')).map(element => element.id || element.textContent) diff --git a/packages/react/src/TreeView/TreeView.tsx b/packages/react/src/TreeView/TreeView.tsx index e3e66b47511..441abd55a38 100644 --- a/packages/react/src/TreeView/TreeView.tsx +++ b/packages/react/src/TreeView/TreeView.tsx @@ -19,6 +19,7 @@ import {useSlots} from '../hooks/useSlots' import {getAccessibleName} from './shared' import {getFirstChildElement} from './useRovingTabIndex' import {useTreeRovingTabIndex} from './useTreeRovingTabIndex' +import {useFocusWithinAttribute} from './useFocusWithinAttribute' import {useTypeahead} from './useTypeahead' import {SkeletonAvatar} from '../SkeletonAvatar' import {SkeletonText} from '../SkeletonText' @@ -141,6 +142,7 @@ const Root: React.FC = ({ }, []) useTreeRovingTabIndex({containerRef, mouseDownRef}) + useFocusWithinAttribute(containerRef) useTypeahead({ containerRef, onFocusChange: element => { diff --git a/packages/react/src/TreeView/useFocusWithinAttribute.ts b/packages/react/src/TreeView/useFocusWithinAttribute.ts new file mode 100644 index 00000000000..8e54b8bcdac --- /dev/null +++ b/packages/react/src/TreeView/useFocusWithinAttribute.ts @@ -0,0 +1,33 @@ +import type React from 'react' +import {useEffect} from 'react' + +/** + * Mirrors `:focus-within` onto the tree root as `data-focus-within`. + * + * WebKit clears and re-sets `:focus-within` up the ancestor chain on every focus move, so a + * `:focus-within` rule on the root restyles every level line on each arrow key press. This + * attribute only changes when focus enters or leaves the tree. It is written to the DOM + * directly so the tree doesn't re-render. + */ +export function useFocusWithinAttribute(containerRef: React.RefObject) { + useEffect(() => { + const root = containerRef.current + if (!root) return + + const sync = (event?: FocusEvent) => { + const focused = event + ? event.type === 'focusin' || root.contains(event.relatedTarget as Node | null) + : root.matches(':focus-within') + root.toggleAttribute('data-focus-within', focused) + } + + sync() + root.addEventListener('focusin', sync) + root.addEventListener('focusout', sync) + return () => { + root.removeEventListener('focusin', sync) + root.removeEventListener('focusout', sync) + root.removeAttribute('data-focus-within') + } + }, [containerRef]) +}