diff --git a/.changeset/performance-inp-optimizations.md b/.changeset/performance-inp-optimizations.md new file mode 100644 index 00000000000..387386ad6d5 --- /dev/null +++ b/.changeset/performance-inp-optimizations.md @@ -0,0 +1,14 @@ +--- +"@primer/react": patch +--- + +Performance: Optimize CSS `:has()` selectors and JavaScript hooks for improved INP + +- Scope CSS `:has()` selectors to direct children across Dialog, PageHeader, ActionList, Banner, ButtonGroup, AvatarStack, Breadcrumbs, SegmentedControl, TreeView, and SelectPanel +- Add requestAnimationFrame throttling to `useResizeObserver` (first callback immediate, subsequent throttled) +- Add anchor element ResizeObserver to `useAnchoredPosition` for repositioning on anchor resize +- Add `useTreeItemCache` hook for caching TreeView DOM queries with MutationObserver invalidation +- Throttle MutationObserver callbacks in AvatarStack and Announce components +- Add rAF throttling to `useOverflow` hook +- Optimize `hasInteractiveNodes` utility with querySelectorAll and attribute-first checks +- Split Autocomplete context to prevent Overlay re-renders during typing diff --git a/.github/instructions/css.instructions.md b/.github/instructions/css.instructions.md index f40aee8e99d..7ca178910fd 100644 --- a/.github/instructions/css.instructions.md +++ b/.github/instructions/css.instructions.md @@ -10,4 +10,73 @@ If the file ends with `*.module.css`, it is a CSS Module. ## General Conventions -- After making a change to a file, use `npx stylelint -q --rd --fix` with a path to the file changed to make sure the code lints +- After making a change to a file, use `npx stylelint -q --rd --fix` with a path to the file changed to make sure the code lints. + +## Performance and Web Vitals Analysis (Required) + +When creating or modifying CSS, you must **explicitly analyze and account for impact on Core Web Vitals**. Treat this as a first-class requirement, not an afterthought. + +### Metrics to Consider + +Evaluate changes against the following metrics and call out any risks: + +- **LCP (Largest Contentful Paint)** + + - Risk factors: render-blocking styles, large above-the-fold background images, heavy font usage, complex selectors on critical elements. + +- **CLS (Cumulative Layout Shift)** + + - Risk factors: late-loading fonts, size-less images/media, conditional styles that affect layout after initial render, JS-driven class toggles that change dimensions. + +- **INP (Interaction to Next Paint)** + + - Risk factors: expensive selector matching, `:has()` on large subtrees, frequent style recalculation triggers, deep descendant selectors on interactive paths. + +### Required Analysis Checklist + +For each meaningful CSS change, reason through and validate the following: + +- **Selector Cost** + + - Prefer class selectors over tag or attribute selectors. + - Avoid deep descendant chains (`.a .b .c .d`) in large or frequently updated subtrees. + - Use `:has()` only when strictly necessary; assume worst-case cost on large DOMs and justify usage. + +- **Style Recalculation Scope** + + - Consider which DOM nodes are affected when classes or attributes change. + - Avoid selectors that match large portions of the tree when toggling state (e.g., `[data-*] .child`). + +- **Layout and Paint Stability** + + - Do not introduce layout-affecting properties (`width`, `height`, `margin`, `padding`, `display`) that may change after hydration or user interaction unless explicitly intended. + - Prefer transform/opacity for interaction-driven effects. + - Ensure predictable sizing for media, containers, and dynamic content to prevent CLS. + +- **Critical Rendering Path** + + - Avoid adding styles that block first paint or LCP for above-the-fold content. + - Be cautious with large background images, filters, and shadows on critical elements. + +### CSS Modules–Specific Guidance + +- Assume CSS Modules may be used across multiple React roots and hydration boundaries. +- Avoid styles that depend on global cascade ordering or implicit inheritance from non-module CSS. +- Do not rely on runtime-injected styles to correct layout shifts introduced by base styles. + +### Documentation Requirement + +When a change has **any non-trivial performance implication**, include a brief inline comment or PR description that states: + +- Which Web Vital(s) may be affected. +- Why the change is safe, or what tradeoff is being made. +- Any assumptions about DOM size, interaction frequency, or rendering phase (SSR vs client). + +### Examples of High-Risk Patterns (Avoid Unless Justified) + +- `:has()` selectors applied to containers with large or frequently changing subtrees. +- Attribute selectors used as global state flags on high-level containers. +- Layout changes driven by hover, focus, or JS state on large components. +- Font or image-dependent sizing without explicit fallbacks. + +Performance regressions are considered correctness issues. If a change cannot be justified as Web Vitals–safe, it should not be merged without explicit sign-off. diff --git a/packages/react/src/ActionBar/ActionBar.tsx b/packages/react/src/ActionBar/ActionBar.tsx index 7f2745ef218..97d4f02b275 100644 --- a/packages/react/src/ActionBar/ActionBar.tsx +++ b/packages/react/src/ActionBar/ActionBar.tsx @@ -320,6 +320,7 @@ export const ActionBar: React.FC> = prop const moreMenuBtnRef = useRef(null) const containerRef = React.useRef(null) + // ResizeObserver is throttled by default (rAF) for better INP useResizeObserver((resizeObserverEntries: ResizeObserverEntry[]) => { const navWidth = resizeObserverEntries[0].contentRect.width const moreMenuWidth = moreMenuRef.current?.getBoundingClientRect().width ?? 0 diff --git a/packages/react/src/ActionList/ActionList.module.css b/packages/react/src/ActionList/ActionList.module.css index f8007fb3e63..c6d178d6cae 100644 --- a/packages/react/src/ActionList/ActionList.module.css +++ b/packages/react/src/ActionList/ActionList.module.css @@ -86,7 +86,14 @@ display: none; } - /* if a list has a mix of items with and without descriptions, reset the label font-weight to normal */ + /* + * If a list has a mix of items with and without descriptions, reset the label font-weight. + * NOTE: Uses double descendant :has() - traverses list twice per recalc. + * This is acceptable because: + * 1. ActionLists are typically small (10-50 items) + * 2. Alternative would require JS to detect mixed state and set data attribute + * 3. The visual impact is subtle (font-weight change), not critical styling + */ &:has([data-has-description='true']):has([data-has-description='false']) { & .ItemLabel { font-weight: var(--base-text-weight-normal); @@ -121,7 +128,8 @@ } } - &:not(:has([aria-disabled], [disabled]), [data-has-subitem='true']) { + /* PERFORMANCE: Use data-disabled on
  • instead of :has([aria-disabled], [disabled]) which scans descendants */ + &:not([aria-disabled], [data-disabled='true'], [data-has-subitem='true']) { @media (hover: hover) { &:hover, &:active { @@ -276,8 +284,8 @@ } } - &:where([data-loading='true']), - &:has([data-loading='true']) { + /* PERFORMANCE: data-loading is set on the
  • by JS, avoiding :has() descendant scan */ + &:where([data-loading='true']) { & * { color: var(--fgColor-muted); } @@ -322,10 +330,9 @@ } } - /* disabled */ - + /* PERFORMANCE: data-disabled is set on the
  • by JS, avoiding :has() descendant scan */ &[aria-disabled='true'], - &:has([aria-disabled='true'], [disabled]) { + &[data-disabled='true'] { & .ActionListContent * { color: var(--control-fgColor-disabled); } @@ -366,7 +373,8 @@ } /* When TrailingAction is in loading state, keep labels and descriptions accessible */ - &:has(.TrailingAction [data-loading='true']):not([aria-disabled='true']) { + /* PERFORMANCE: scoped to direct child TrailingAction */ + &:has(> .TrailingAction[data-loading='true']):not([aria-disabled='true']) { /* Ensure labels and descriptions maintain accessibility contrast */ & .ItemLabel { color: var(--fgColor-default); @@ -540,7 +548,11 @@ display: none; } - /* show active indicator on parent collapse if child is active */ + /* + * Show active indicator on parent collapse if child is active. + * NOTE: Uses adjacent sibling + descendant :has() - SubGroup is typically small (few nav items). + * This is acceptable because scope is limited to the collapsed SubGroup's children. + */ &:has(+ .SubGroup [data-active='true']) { background: var(--control-transparent-bgColor-selected); @@ -637,6 +649,7 @@ default block */ word-break: normal; } + /* NOTE: Uses descendant :has() - scope is just this item's children (label + description). Acceptable. */ &:has([data-truncate='true']) { & .ItemLabel { flex: 1 0 auto; @@ -713,7 +726,8 @@ span wrapping svg or text */ height: 100%; /* Preserve width consistency when loading state is active for text buttons only */ - &[data-loading='true']:has([data-component='buttonContent']) { + /* PERFORMANCE: scoped to direct child for O(1) lookup */ + &[data-loading='true']:has(> [data-component='buttonContent']) { /* Double the left padding to compensate for missing right padding */ padding: 0 0 0 calc(var(--base-size-12) * 2); @@ -731,6 +745,11 @@ span wrapping svg or text */ } } +/* + * NOTE: Uses descendant :has() - VisualComponent is nested inside Tooltip > button, + * which is 3 levels deep (> * > * > .TrailingVisual). This is acceptable since + * the search is scoped to this small wrapper element's subtree. + */ .InactiveButtonWrap { &:has(.TrailingVisual) { grid-area: trailingVisual; diff --git a/packages/react/src/ActionList/Group.module.css b/packages/react/src/ActionList/Group.module.css index 201d3086b86..761b9ec15c4 100644 --- a/packages/react/src/ActionList/Group.module.css +++ b/packages/react/src/ActionList/Group.module.css @@ -4,7 +4,11 @@ &:not(:first-child) { margin-block-start: var(--base-size-8); - /* If somebody tries to pass the `title` prop AND a `NavList.GroupHeading` as a child, hide the `ActionList.GroupHeading */ + /* + * If somebody tries to pass the `title` prop AND a `NavList.GroupHeading` as a child, hide the `ActionList.GroupHeading`. + * NOTE: Uses descendant :has() - this is an edge case handler for developer errors. + * Scope is limited to group's children, not entire list. Acceptable performance cost. + */ /* stylelint-disable-next-line selector-max-specificity */ &:has(.GroupHeadingWrap + ul > .GroupHeadingWrap) { /* stylelint-disable-next-line selector-max-specificity */ diff --git a/packages/react/src/ActionList/Item.tsx b/packages/react/src/ActionList/Item.tsx index ac917743fd5..90122ccf5b3 100644 --- a/packages/react/src/ActionList/Item.tsx +++ b/packages/react/src/ActionList/Item.tsx @@ -256,6 +256,8 @@ const UnwrappedItem = ( data-variant={variant === 'danger' ? variant : undefined} data-active={active ? true : undefined} data-inactive={inactiveText ? true : undefined} + data-loading={loading && !inactive ? true : undefined} + data-disabled={disabled ? true : undefined} data-has-subitem={slots.subItem ? true : undefined} data-has-description={slots.description ? true : false} className={clsx(classes.ActionListItem, className)} diff --git a/packages/react/src/ActionList/TrailingAction.tsx b/packages/react/src/ActionList/TrailingAction.tsx index 933c3a6f584..bb1824cdedf 100644 --- a/packages/react/src/ActionList/TrailingAction.tsx +++ b/packages/react/src/ActionList/TrailingAction.tsx @@ -31,7 +31,7 @@ export type ActionListTrailingActionProps = ElementProps & { export const TrailingAction = forwardRef( ({as = 'button', icon, label, href = null, className, style, loading, ...props}, forwardedRef) => { return ( - + {icon ? ( > = ( }, []) const id = useId(idProp) + // Base context: refs, IDs, menu visibility, and callbacks + // Changes when menu opens/closes or selection changes, but NOT on every keystroke + const autocompleteContextValue = useMemo( + () => ({ + activeDescendantRef, + id, + inputRef, + scrollContainerRef, + selectedItemLength, + setAutocompleteSuggestion, + setInputValue, + setIsMenuDirectlyActivated, + setShowMenu, + setSelectedItemLength, + showMenu, + }), + [ + id, + selectedItemLength, + setAutocompleteSuggestion, + setInputValue, + setIsMenuDirectlyActivated, + setShowMenu, + setSelectedItemLength, + showMenu, + ], + ) + + // Input state context: values that change on every keystroke + // Split to prevent Overlay from re-rendering during typing + const autocompleteInputContextValue = useMemo( + () => ({ + autocompleteSuggestion, + inputValue, + isMenuDirectlyActivated, + }), + [autocompleteSuggestion, inputValue, isMenuDirectlyActivated], + ) + + // Deferred input value for expensive operations like filtering + // Menu subscribes to this instead of inputValue to avoid re-rendering on every keystroke + const deferredInputValue = useDeferredValue(inputValue) + const autocompleteDeferredInputContextValue = useMemo(() => ({deferredInputValue}), [deferredInputValue]) + return ( - - {children} + + + + {children} + + ) } diff --git a/packages/react/src/Autocomplete/AutocompleteContext.tsx b/packages/react/src/Autocomplete/AutocompleteContext.tsx index 524ae7074db..e25455664fe 100644 --- a/packages/react/src/Autocomplete/AutocompleteContext.tsx +++ b/packages/react/src/Autocomplete/AutocompleteContext.tsx @@ -1,13 +1,15 @@ import {createContext} from 'react' +/** + * Base context containing refs, stable IDs, menu visibility state, and callbacks. + * This context changes when menu opens/closes or selection changes, but NOT on every keystroke. + * Consumers like AutocompleteOverlay that don't need input text should use only this context. + */ export const AutocompleteContext = createContext<{ activeDescendantRef: React.MutableRefObject - autocompleteSuggestion: string // TODO: consider changing `id` to `listboxId` because we're just using it to associate the input and combobox with the listbox id: string inputRef: React.MutableRefObject - inputValue: string - isMenuDirectlyActivated: boolean scrollContainerRef: React.MutableRefObject selectedItemLength: number setAutocompleteSuggestion: (value: string) => void @@ -17,3 +19,23 @@ export const AutocompleteContext = createContext<{ setShowMenu: (value: boolean) => void showMenu: boolean } | null>(null) + +/** + * Input-related state that changes on every keystroke. + * Only AutocompleteInput needs this for immediate text display and suggestion highlighting. + */ +export const AutocompleteInputContext = createContext<{ + autocompleteSuggestion: string + inputValue: string + isMenuDirectlyActivated: boolean +} | null>(null) + +/** + * Deferred input value for expensive operations like filtering. + * Uses React's useDeferredValue to allow typing to remain responsive while + * filtering large lists at lower priority. + * AutocompleteMenu uses this to avoid blocking keystrokes during filtering. + */ +export const AutocompleteDeferredInputContext = createContext<{ + deferredInputValue: string +} | null>(null) diff --git a/packages/react/src/Autocomplete/AutocompleteInput.tsx b/packages/react/src/Autocomplete/AutocompleteInput.tsx index 4b0314884cc..61b2905e5ba 100644 --- a/packages/react/src/Autocomplete/AutocompleteInput.tsx +++ b/packages/react/src/Autocomplete/AutocompleteInput.tsx @@ -1,7 +1,7 @@ import type {ChangeEventHandler, FocusEventHandler, KeyboardEventHandler} from 'react' import React, {useCallback, useContext, useEffect, useState} from 'react' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' -import {AutocompleteContext} from './AutocompleteContext' +import {AutocompleteContext, AutocompleteInputContext} from './AutocompleteContext' import TextInput from '../TextInput' import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef' import type {ComponentProps} from '../utils/types' @@ -37,20 +37,12 @@ const AutocompleteInput = React.forwardRef( forwardedRef, ) => { const autocompleteContext = useContext(AutocompleteContext) - if (autocompleteContext === null) { + const inputContext = useContext(AutocompleteInputContext) + if (autocompleteContext === null || inputContext === null) { throw new Error('AutocompleteContext returned null values') } - const { - activeDescendantRef, - autocompleteSuggestion = '', - id, - inputRef, - inputValue = '', - isMenuDirectlyActivated, - setInputValue, - setShowMenu, - showMenu, - } = autocompleteContext + const {activeDescendantRef, id, inputRef, setInputValue, setShowMenu, showMenu} = autocompleteContext + const {autocompleteSuggestion = '', inputValue = '', isMenuDirectlyActivated} = inputContext useRefObjectAsForwardedRef(forwardedRef, inputRef) const [highlightRemainingText, setHighlightRemainingText] = useState(true) const {safeSetTimeout} = useSafeTimeout() diff --git a/packages/react/src/Autocomplete/AutocompleteMenu.tsx b/packages/react/src/Autocomplete/AutocompleteMenu.tsx index bd987d0068d..1e53386750b 100644 --- a/packages/react/src/Autocomplete/AutocompleteMenu.tsx +++ b/packages/react/src/Autocomplete/AutocompleteMenu.tsx @@ -9,7 +9,7 @@ import {useFocusZone} from '../hooks/useFocusZone' import type {ComponentProps, MandateProps, AriaRole} from '../utils/types' import Spinner from '../Spinner' import {useId} from '../hooks/useId' -import {AutocompleteContext} from './AutocompleteContext' +import {AutocompleteContext, AutocompleteDeferredInputContext} from './AutocompleteContext' import type {IconProps} from '@primer/octicons-react' import {PlusIcon} from '@primer/octicons-react' import VisuallyHidden from '../_VisuallyHidden' @@ -132,14 +132,14 @@ const debounceAnnouncement = debounce((announcement: string) => { function AutocompleteMenu(props: AutocompleteMenuInternalProps) { const autocompleteContext = useContext(AutocompleteContext) - if (autocompleteContext === null) { + const deferredInputContext = useContext(AutocompleteDeferredInputContext) + if (autocompleteContext === null || deferredInputContext === null) { throw new Error('AutocompleteContext returned null values') } const { activeDescendantRef, id, inputRef, - inputValue = '', scrollContainerRef, setAutocompleteSuggestion, setShowMenu, @@ -148,6 +148,9 @@ function AutocompleteMenu(props: AutocompleteMe setSelectedItemLength, showMenu, } = autocompleteContext + // Use deferred input value to avoid re-rendering on every keystroke + // This allows filtering large lists without blocking typing + const {deferredInputValue} = deferredInputContext const { items, selectedItemIds, @@ -226,9 +229,9 @@ function AutocompleteMenu(props: AutocompleteMe const sortedAndFilteredItemsToRender = useMemo( () => selectableItems - .filter(filterFn ? filterFn : getDefaultItemFilter(inputValue)) + .filter(filterFn ? filterFn : getDefaultItemFilter(deferredInputValue)) .sort((a, b) => itemSortOrderData[a.id] - itemSortOrderData[b.id]), - [selectableItems, itemSortOrderData, filterFn, inputValue], + [selectableItems, itemSortOrderData, filterFn, deferredInputValue], ) const allItemsToRender = useMemo( @@ -311,12 +314,14 @@ function AutocompleteMenu(props: AutocompleteMe ) useEffect(() => { - if (highlightedItem?.text?.startsWith(inputValue) && !selectedItemIds.includes(highlightedItem.id)) { + // Use deferredInputValue to avoid running this effect on every keystroke + // The Input component guards against stale suggestions + if (highlightedItem?.text?.startsWith(deferredInputValue) && !selectedItemIds.includes(highlightedItem.id)) { setAutocompleteSuggestion(highlightedItem.text) } else { setAutocompleteSuggestion('') } - }, [highlightedItem, inputValue, selectedItemIds, setAutocompleteSuggestion]) + }, [highlightedItem, deferredInputValue, selectedItemIds, setAutocompleteSuggestion]) useEffect(() => { const itemIdSortResult = [...sortedItemIds].sort( diff --git a/packages/react/src/AvatarStack/AvatarStack.module.css b/packages/react/src/AvatarStack/AvatarStack.module.css index b455ec447c8..5adc0fa958e 100644 --- a/packages/react/src/AvatarStack/AvatarStack.module.css +++ b/packages/react/src/AvatarStack/AvatarStack.module.css @@ -143,7 +143,8 @@ box-shadow: 0 0 0 var(--avatar-border-width) transparent; } - &:not([data-component='Avatar']):not(:has([data-square])) { + /* PERFORMANCE: Avatar with data-square is direct child, scope with > for O(1) lookup */ + &:not([data-component='Avatar']):not(:has(> [data-square])) { border-radius: 50%; } diff --git a/packages/react/src/AvatarStack/AvatarStack.tsx b/packages/react/src/AvatarStack/AvatarStack.tsx index ae400a90876..8d67d7249b0 100644 --- a/packages/react/src/AvatarStack/AvatarStack.tsx +++ b/packages/react/src/AvatarStack/AvatarStack.tsx @@ -120,7 +120,19 @@ const AvatarStack = ({ setHasInteractiveChildren(hasInteractiveNodes(stackContainer.current)) } - const observer = new MutationObserver(interactiveChildren) + // Track pending frame to throttle MutationObserver callbacks + let pendingFrame: number | null = null + const throttledInteractiveChildren = () => { + if (pendingFrame !== null) { + cancelAnimationFrame(pendingFrame) + } + pendingFrame = requestAnimationFrame(() => { + pendingFrame = null + interactiveChildren() + }) + } + + const observer = new MutationObserver(throttledInteractiveChildren) observer.observe(stackContainer.current, {childList: true}) @@ -129,6 +141,9 @@ const AvatarStack = ({ return () => { observer.disconnect() + if (pendingFrame !== null) { + cancelAnimationFrame(pendingFrame) + } } } }, []) diff --git a/packages/react/src/Banner/Banner.module.css b/packages/react/src/Banner/Banner.module.css index 07b04346567..5377ecead6b 100644 --- a/packages/react/src/Banner/Banner.module.css +++ b/packages/react/src/Banner/Banner.module.css @@ -119,7 +119,8 @@ margin-block: var(--base-size-8); } -.Banner[data-title-hidden]:not(:has(.BannerActions)) .BannerContent { +/* PERFORMANCE: BannerActions is inside BannerContainer, use shallow path > * > for O(1) lookup */ +.Banner[data-title-hidden]:not(:has(> * > .BannerActions)) .BannerContent { margin-block: var(--base-size-6); } @@ -152,8 +153,9 @@ fill: var(--banner-icon-fgColor); } +/* PERFORMANCE: BannerActions is inside BannerContainer, use shallow path > * > for O(1) lookup */ /* stylelint-disable-next-line selector-max-specificity */ -.Banner[data-title-hidden]:not(:has(.BannerActions)) .BannerIcon svg { +.Banner[data-title-hidden]:not(:has(> * > .BannerActions)) .BannerIcon svg { height: var(--base-size-16); } @@ -166,7 +168,8 @@ margin-inline-start: var(--base-size-4); } -:where(.Banner):has(.BannerActions) .BannerDismiss { +/* PERFORMANCE: BannerActions is inside BannerContainer, use shallow path > * > for O(1) lookup */ +:where(.Banner):has(> * > .BannerActions) .BannerDismiss { margin-block: var(--base-size-2); } @@ -211,7 +214,8 @@ display: grid; } - .BannerContainer:has(.BannerActionsContainer) { + /* PERFORMANCE: BannerActionsContainer is direct child of BannerContainer, scope with > for O(1) lookup */ + .BannerContainer:has(> .BannerActionsContainer) { grid-template-rows: auto auto; } diff --git a/packages/react/src/BaseStyles.module.css b/packages/react/src/BaseStyles.module.css index 76259ac05dd..3e6352c92e6 100644 --- a/packages/react/src/BaseStyles.module.css +++ b/packages/react/src/BaseStyles.module.css @@ -60,19 +60,12 @@ details-dialog:focus:not(:focus-visible):not(:global(.focus-visible)) { /* stylelint-disable-next-line primer/colors */ color: var(--BaseStyles-fgColor, var(--fgColor-default)); - /* Global styles for light mode */ - &:has([data-color-mode='light']) { - input & { - color-scheme: light; - } - } - - /* Global styles for dark mode */ - &:has([data-color-mode='dark']) { - input & { - color-scheme: dark; - } - } + /* + * PERFORMANCE: Removed :has([data-color-mode]) selectors that scanned entire DOM. + * Input color-scheme is already handled by global selectors above: + * [data-color-mode='light'] input { color-scheme: light; } + * [data-color-mode='dark'] input { color-scheme: dark; } + */ /* Low-specificity default link styling */ :where(a:not([class*='prc-']):not([class*='PRC-']):not([class*='Primer_Brand__'])) { diff --git a/packages/react/src/Breadcrumbs/Breadcrumbs.module.css b/packages/react/src/Breadcrumbs/Breadcrumbs.module.css index e699fb42587..11f1633894e 100644 --- a/packages/react/src/Breadcrumbs/Breadcrumbs.module.css +++ b/packages/react/src/Breadcrumbs/Breadcrumbs.module.css @@ -117,7 +117,8 @@ list-style: none; /* allow menu items to wrap line */ - &:has(.MenuOverlay) { + /* PERFORMANCE: MenuOverlay is inside MenuDetails > summary sibling, use shallow path */ + &:has(> .MenuDetails .MenuOverlay) { white-space: normal; } diff --git a/packages/react/src/Breadcrumbs/Breadcrumbs.tsx b/packages/react/src/Breadcrumbs/Breadcrumbs.tsx index 5a59efe2f4c..e5353a285a1 100644 --- a/packages/react/src/Breadcrumbs/Breadcrumbs.tsx +++ b/packages/react/src/Breadcrumbs/Breadcrumbs.tsx @@ -188,9 +188,11 @@ function Breadcrumbs({className, children, style, overflow = 'wrap', variant = ' listElement.children.length === childArray.length ) { const listElementArray = Array.from(listElement.children) as HTMLElement[] + // Batch all offsetWidth reads in a single pass to avoid layout thrashing const widths = listElementArray.map(child => child.offsetWidth) setChildArrayWidths(widths) - setRootItemWidth(listElementArray[0].offsetWidth) + // Use first width from the array instead of reading offsetWidth again + setRootItemWidth(widths[0]) } }, [childArray, overflowMenuEnabled]) diff --git a/packages/react/src/Button/ButtonBase.module.css b/packages/react/src/Button/ButtonBase.module.css index 7ff56b63091..7af731290da 100644 --- a/packages/react/src/Button/ButtonBase.module.css +++ b/packages/react/src/Button/ButtonBase.module.css @@ -24,6 +24,7 @@ justify-content: space-between; gap: var(--base-size-8); + /* NOTE: Uses descendant :has() - button has very few children (icon, text, kbd). Acceptable. */ &:has([data-kbd-chord]) { padding-inline-end: var(--base-size-6); } @@ -173,6 +174,7 @@ margin-right: var(--control-large-gap); } + /* NOTE: Uses descendant :has() - button has very few children (icon, text, kbd). Acceptable. */ &:has([data-kbd-chord]) { padding-inline-end: var(--base-size-8); } @@ -580,12 +582,13 @@ } } + /* PERFORMANCE: Use data-no-visuals attribute instead of :has(.Visual) to avoid descendant scan */ [data-a11y-link-underlines='true'] &:where([data-variant='link']) { - &:not(:has(.Visual)) { + &:where([data-no-visuals='true']) { text-decoration: underline; } - &:has(.Visual) { + &:not([data-no-visuals='true']) { background-image: linear-gradient(to right, currentColor, currentColor); background-size: 100% 1.5px; background-position: 0 calc(100% - 2px); @@ -598,12 +601,12 @@ } [data-a11y-link-underlines='false'] &:where([data-variant='link']) { - &:not(:has(.Visual)) { + &:where([data-no-visuals='true']) { text-decoration: none; background-image: none; } - &:has(.Visual) { + &:not([data-no-visuals='true']) { background-image: none; } } @@ -632,8 +635,12 @@ } } - /* Icon-only + Counter */ - + /* + * Icon-only + Counter + * NOTE: Uses descendant :has() - leadingVisual and text are nested inside + * buttonContent wrapper. This is acceptable as the search is scoped to + * this single button element's subtree (small DOM). + */ &:where([data-has-count]):has([data-component='leadingVisual']):not(:has([data-component='text'])) { /* stylelint-disable-next-line primer/spacing */ padding-inline: var(--control-medium-paddingInline-condensed); diff --git a/packages/react/src/ButtonGroup/ButtonGroup.module.css b/packages/react/src/ButtonGroup/ButtonGroup.module.css index 7c289b69544..490b13cf49c 100644 --- a/packages/react/src/ButtonGroup/ButtonGroup.module.css +++ b/packages/react/src/ButtonGroup/ButtonGroup.module.css @@ -38,7 +38,8 @@ } /* this is a workaround until portal based tooltips are fully removed from dotcom */ - &:has(div:last-child:empty) { + /* PERFORMANCE: Scope to direct child for O(1) lookup */ + &:has(> div:last-child:empty) { button, a { border-radius: var(--borderRadius-medium); diff --git a/packages/react/src/Dialog/Dialog.module.css b/packages/react/src/Dialog/Dialog.module.css index d750854c158..65bc9b6516e 100644 --- a/packages/react/src/Dialog/Dialog.module.css +++ b/packages/react/src/Dialog/Dialog.module.css @@ -229,7 +229,12 @@ } } -body:has(.Dialog.DisableScroll) { +/* + * PERFORMANCE: Using a direct class on body instead of body:has(.Dialog.DisableScroll) + * The :has() selector forces the browser to scan the entire DOM on every style recalc, + * which is O(n) where n is the number of DOM nodes. This is expensive on large pages. + */ +body:global(.DialogScrollDisabled) { /* stylelint-disable-next-line primer/spacing */ padding-right: var(--prc-dialog-scrollgutter) !important; overflow: hidden !important; @@ -244,8 +249,9 @@ Add a border between the body and footer if: - the dialog has a footer - the dialog has a body that can scroll - the browser supports the `animation-timeline` property and its `scroll()` function +PERFORMANCE: Footer is a direct child of Dialog, scope with > for O(1) lookup */ -.Dialog:has(.Footer) { +.Dialog:has(> .Footer) { --can-scroll: 0; .DialogOverflowWrapper { diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index 0381c445e58..2fe0aac7dc3 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -296,6 +296,14 @@ const _Dialog = React.forwardRef { + document.body.classList.remove('DialogScrollDisabled') + document.body.style.removeProperty('--prc-dialog-scrollgutter') + } }, []) const header = slots.header ?? (renderHeader ?? DefaultHeader)(defaultedProps) diff --git a/packages/react/src/PageHeader/PageHeader.module.css b/packages/react/src/PageHeader/PageHeader.module.css index 92486619fe8..7baf2228b4c 100644 --- a/packages/react/src/PageHeader/PageHeader.module.css +++ b/packages/react/src/PageHeader/PageHeader.module.css @@ -33,8 +33,9 @@ line-height is calculated with calc(height/font-size) and the below numbers are from @primer/primitives. --custom-font-size, --custom-line-height, --custom-font-weight are custom properties that can be used to override the below values. We don't want these values to be overridden but still want to allow consumers to override them if needed. + PERFORMANCE: Scoped :has() to direct child (>) for O(1) lookup instead of descendant scan. */ - &:has([data-component='TitleArea'][data-size-variant='large']) { + &:has(> [data-component='TitleArea'][data-size-variant='large']) { font-size: var(--custom-font-size, var(--text-title-size-large, 2rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); /* calc(48/32) */ @@ -42,7 +43,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); } - &:has([data-component='TitleArea'][data-size-variant='medium']) { + &:has(> [data-component='TitleArea'][data-size-variant='medium']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-semibold, 600)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); /* calc(32/20) */ @@ -50,7 +51,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); } - &:has([data-component='TitleArea'][data-size-variant='subtitle']) { + &:has(> [data-component='TitleArea'][data-size-variant='subtitle']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); /* calc(32/20) */ @@ -60,7 +61,7 @@ /* Responsive size variants */ @media (--viewportRange-narrow) { - &:has([data-component='TitleArea'][data-size-variant-narrow='large']) { + &:has(> [data-component='TitleArea'][data-size-variant-narrow='large']) { font-size: var(--custom-font-size, var(--text-title-size-large, 2rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); @@ -68,7 +69,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); } - &:has([data-component='TitleArea'][data-size-variant-narrow='medium']) { + &:has(> [data-component='TitleArea'][data-size-variant-narrow='medium']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-semibold, 600)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -76,7 +77,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); } - &:has([data-component='TitleArea'][data-size-variant-narrow='subtitle']) { + &:has(> [data-component='TitleArea'][data-size-variant-narrow='subtitle']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -86,7 +87,7 @@ } @media (--viewportRange-regular) { - &:has([data-component='TitleArea'][data-size-variant-regular='large']) { + &:has(> [data-component='TitleArea'][data-size-variant-regular='large']) { font-size: var(--custom-font-size, var(--text-title-size-large, 2rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); @@ -94,7 +95,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); } - &:has([data-component='TitleArea'][data-size-variant-regular='medium']) { + &:has(> [data-component='TitleArea'][data-size-variant-regular='medium']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-semibold, 600)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -102,7 +103,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); } - &:has([data-component='TitleArea'][data-size-variant-regular='subtitle']) { + &:has(> [data-component='TitleArea'][data-size-variant-regular='subtitle']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -112,7 +113,7 @@ } @media (--viewportRange-wide) { - &:has([data-component='TitleArea'][data-size-variant-wide='large']) { + &:has(> [data-component='TitleArea'][data-size-variant-wide='large']) { font-size: var(--custom-font-size, var(--text-title-size-large, 2rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); @@ -120,7 +121,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); } - &:has([data-component='TitleArea'][data-size-variant-wide='medium']) { + &:has(> [data-component='TitleArea'][data-size-variant-wide='medium']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-semibold, 600)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -128,7 +129,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); } - &:has([data-component='TitleArea'][data-size-variant-wide='subtitle']) { + &:has(> [data-component='TitleArea'][data-size-variant-wide='subtitle']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -137,28 +138,28 @@ } } - &[data-has-border='true']:has([data-component='PH_Navigation'][data-hidden-all]), - &[data-has-border='true']:not(:has([data-component='PH_Navigation'])) { + &[data-has-border='true']:has(> [data-component='PH_Navigation'][data-hidden-all]), + &[data-has-border='true']:not(:has(> [data-component='PH_Navigation'])) { border-block-end: var(--borderWidth-thin) solid var(--borderColor-default); padding-block-end: var(--base-size-8); } @media (--viewportRange-narrow) { - &[data-has-border='true']:has([data-component='PH_Navigation'][data-hidden-narrow]) { + &[data-has-border='true']:has(> [data-component='PH_Navigation'][data-hidden-narrow]) { border-block-end: var(--borderWidth-thin) solid var(--borderColor-default); padding-block-end: var(--base-size-8); } } @media (--viewportRange-regular) { - &[data-has-border='true']:has([data-component='PH_Navigation'][data-hidden-regular]) { + &[data-has-border='true']:has(> [data-component='PH_Navigation'][data-hidden-regular]) { border-block-end: var(--borderWidth-thin) solid var(--borderColor-default); padding-block-end: var(--base-size-8); } } @media (--viewportRange-wide) { - &[data-has-border='true']:has([data-component='PH_Navigation'][data-hidden-wide]) { + &[data-has-border='true']:has(> [data-component='PH_Navigation'][data-hidden-wide]) { border-block-end: var(--borderWidth-thin) solid var(--borderColor-default); padding-block-end: var(--base-size-8); } diff --git a/packages/react/src/SegmentedControl/SegmentedControl.module.css b/packages/react/src/SegmentedControl/SegmentedControl.module.css index 22b80ae8f33..0d936ae4637 100644 --- a/packages/react/src/SegmentedControl/SegmentedControl.module.css +++ b/packages/react/src/SegmentedControl/SegmentedControl.module.css @@ -187,7 +187,8 @@ } } - &:focus-within:has(:focus-visible) { + /* PERFORMANCE: Button is direct child of Item, scope with > for O(1) lookup */ + &:focus-within:has(> :focus-visible) { background-color: transparent; } diff --git a/packages/react/src/Timeline/Timeline.module.css b/packages/react/src/Timeline/Timeline.module.css index 608df26e2eb..129e352d355 100644 --- a/packages/react/src/Timeline/Timeline.module.css +++ b/packages/react/src/Timeline/Timeline.module.css @@ -105,6 +105,7 @@ border: 0; border-top: var(--borderWidth-thicker) solid var(--borderColor-default); + /* NOTE: Uses adjacent sibling :has() - checks only the next sibling. O(1) complexity. */ &:has(+ [data-condensed]) { margin-bottom: calc(-1 * var(--base-size-12)); } diff --git a/packages/react/src/TreeView/TreeView.module.css b/packages/react/src/TreeView/TreeView.module.css index 86de774932b..17c881e4407 100644 --- a/packages/react/src/TreeView/TreeView.module.css +++ b/packages/react/src/TreeView/TreeView.module.css @@ -67,6 +67,11 @@ --min-item-height: 2.75rem; } + /* + * NOTE: Uses descendant :has() - TreeViewItemSkeleton is nested inside + * TreeViewItemContent > TreeViewItemContentText, not a direct child. + * This is acceptable as the search is scoped to this element's subtree. + */ &:has(.TreeViewItemSkeleton):hover { cursor: default; background-color: transparent; diff --git a/packages/react/src/TreeView/useRovingTabIndex.ts b/packages/react/src/TreeView/useRovingTabIndex.ts index ff6af01870d..98a7dbbfa6b 100644 --- a/packages/react/src/TreeView/useRovingTabIndex.ts +++ b/packages/react/src/TreeView/useRovingTabIndex.ts @@ -187,7 +187,9 @@ export function getFirstElement(element: HTMLElement): HTMLElement | undefined { export function getLastElement(element: HTMLElement): HTMLElement | undefined { const root = element.closest('[role=tree]') - const items = Array.from(root?.querySelectorAll('[role=treeitem]') || []) + if (!root) return + + const items = Array.from(root.querySelectorAll('[role=treeitem]')) // If there are no items, return undefined if (items.length === 0) return diff --git a/packages/react/src/TreeView/useTreeItemCache.ts b/packages/react/src/TreeView/useTreeItemCache.ts new file mode 100644 index 00000000000..1069129b554 --- /dev/null +++ b/packages/react/src/TreeView/useTreeItemCache.ts @@ -0,0 +1,50 @@ +import React from 'react' + +/** + * A hook that caches tree items to avoid expensive querySelectorAll calls on every keypress. + * The cache is invalidated when the tree structure changes (via MutationObserver). + * + * PERFORMANCE: This is critical for INP because querySelectorAll('[role="treeitem"]') + * on large trees can take 10-50ms, which directly blocks user input response. + */ +export function useTreeItemCache(containerRef: React.RefObject) { + const cacheRef = React.useRef([]) + + // Invalidate cache when tree structure changes + React.useEffect(() => { + const container = containerRef.current + if (!container) return + + // Watch for structural changes (items added/removed, expanded/collapsed) + const observer = new MutationObserver(() => { + cacheRef.current = [] + }) + observer.observe(container, { + childList: true, + subtree: true, + attributes: true, + attributeFilter: ['aria-expanded', 'role'], + }) + + // Clear cache on mount to ensure fresh query + cacheRef.current = [] + + return () => observer.disconnect() + }, [containerRef]) + + const getTreeItems = React.useCallback((): HTMLElement[] => { + const container = containerRef.current + if (!container) return [] + + // Return cached items if valid + if (cacheRef.current.length > 0) { + return cacheRef.current + } + + // Rebuild cache + cacheRef.current = Array.from(container.querySelectorAll('[role="treeitem"]')) as HTMLElement[] + return cacheRef.current + }, [containerRef]) + + return {getTreeItems} +} diff --git a/packages/react/src/TreeView/useTypeahead.ts b/packages/react/src/TreeView/useTypeahead.ts index cfd5f35f6d4..128b6f967eb 100644 --- a/packages/react/src/TreeView/useTypeahead.ts +++ b/packages/react/src/TreeView/useTypeahead.ts @@ -1,6 +1,7 @@ import React from 'react' import useSafeTimeout from '../hooks/useSafeTimeout' import {getAccessibleName} from './shared' +import {useTreeItemCache} from './useTreeItemCache' type TypeaheadOptions = { containerRef: React.RefObject @@ -12,6 +13,7 @@ export function useTypeahead({containerRef, onFocusChange}: TypeaheadOptions) { const timeoutRef = React.useRef(0) const onFocusChangeRef = React.useRef(onFocusChange) const {safeSetTimeout, safeClearTimeout} = useSafeTimeout() + const {getTreeItems} = useTreeItemCache(containerRef) // Update the ref when the callback changes React.useEffect(() => { @@ -25,10 +27,9 @@ export function useTypeahead({containerRef, onFocusChange}: TypeaheadOptions) { if (!searchValue) return if (!containerRef.current) return - const container = containerRef.current - // Get focusable elements - const elements = Array.from(container.querySelectorAll('[role="treeitem"]')) + // PERFORMANCE: Use cached tree items instead of querySelectorAll on every keypress + const elements = getTreeItems() // Get the index of active element const activeIndex = elements.findIndex(element => element === document.activeElement) @@ -53,7 +54,7 @@ export function useTypeahead({containerRef, onFocusChange}: TypeaheadOptions) { onFocusChangeRef.current(nextElement) } }, - [containerRef], + [containerRef, getTreeItems], ) // Update the search value when the user types diff --git a/packages/react/src/UnderlineNav/UnderlineNav.tsx b/packages/react/src/UnderlineNav/UnderlineNav.tsx index c0a252d3ffe..58ad30d2aaa 100644 --- a/packages/react/src/UnderlineNav/UnderlineNav.tsx +++ b/packages/react/src/UnderlineNav/UnderlineNav.tsx @@ -291,6 +291,7 @@ export const UnderlineNav = forwardRef( useOnOutsideClick({onClickOutside: closeOverlay, containerRef, ignoreClickRefs: [moreMenuBtnRef]}) + // ResizeObserver callbacks are now throttled with rAF for better INP useResizeObserver((resizeObserverEntries: ResizeObserverEntry[]) => { const navWidth = resizeObserverEntries[0].contentRect.width const moreMenuWidth = moreMenuRef.current?.getBoundingClientRect().width ?? 0 diff --git a/packages/react/src/UnderlineNav/UnderlineNavItem.tsx b/packages/react/src/UnderlineNav/UnderlineNavItem.tsx index 3341f677e11..29c6750a209 100644 --- a/packages/react/src/UnderlineNav/UnderlineNavItem.tsx +++ b/packages/react/src/UnderlineNav/UnderlineNavItem.tsx @@ -91,11 +91,15 @@ export const UnderlineNavItem = forwardRef( ) as HTMLElement const text = content.textContent as string - const iconWidthWithMargin = icon - ? icon.getBoundingClientRect().width + - Number(getComputedStyle(icon).marginRight.slice(0, -2)) + - Number(getComputedStyle(icon).marginLeft.slice(0, -2)) - : 0 + let iconWidthWithMargin = 0 + if (icon) { + // Batch all layout reads: getBoundingClientRect and getComputedStyle together + const iconRect = icon.getBoundingClientRect() + const iconStyle = getComputedStyle(icon) + // Use || 0 fallback for edge cases where margin might be 'auto' or non-numeric + iconWidthWithMargin = + iconRect.width + (parseFloat(iconStyle.marginRight) || 0) + (parseFloat(iconStyle.marginLeft) || 0) + } setChildrenWidth({text, width: domRect.width}) setNoIconChildrenWidth({text, width: domRect.width - iconWidthWithMargin}) diff --git a/packages/react/src/experimental/SelectPanel2/SelectPanel.module.css b/packages/react/src/experimental/SelectPanel2/SelectPanel.module.css index 7689c6c9261..3ea6ebf6005 100644 --- a/packages/react/src/experimental/SelectPanel2/SelectPanel.module.css +++ b/packages/react/src/experimental/SelectPanel2/SelectPanel.module.css @@ -112,7 +112,12 @@ .TextInput { padding-left: var(--base-size-8) !important; - /* stylelint-disable-next-line selector-class-pattern */ + /* + * NOTE: Uses descendant :has() - input is not direct child of TextInputWrapper + * due to TextInputInnerVisualSlot wrappers. This is acceptable performance + * as the search is scoped to this single element's subtree. + */ + /* stylelint-disable-next-line selector-class-pattern -- global class from TextInput */ &:has(input:placeholder-shown) :global(.TextInput-action) { display: none; } diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx index 575593ece7d..ef6975c3d18 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx @@ -144,19 +144,15 @@ const UnderlinePanels: FCWithSlotMarker = ({ // when the wrapper resizes, check if the icons should be visible // by comparing the wrapper width to the list width - useResizeObserver( - (resizeObserverEntries: ResizeObserverEntry[]) => { - if (!tabsHaveIcons) { - return - } + useResizeObserver((resizeObserverEntries: ResizeObserverEntry[]) => { + if (!tabsHaveIcons) { + return + } - const wrapperWidth = resizeObserverEntries[0].contentRect.width + const wrapperWidth = resizeObserverEntries[0].contentRect.width - setIconsVisible(wrapperWidth > listWidth) - }, - wrapperRef, - [], - ) + setIconsVisible(wrapperWidth > listWidth) + }, wrapperRef) if (__DEV__) { const selectedTabs = tabs.filter(tab => { diff --git a/packages/react/src/hooks/useAnchoredPosition.ts b/packages/react/src/hooks/useAnchoredPosition.ts index 32777aad1d7..ed932272e74 100644 --- a/packages/react/src/hooks/useAnchoredPosition.ts +++ b/packages/react/src/hooks/useAnchoredPosition.ts @@ -2,7 +2,6 @@ import React from 'react' import {getAnchoredPosition} from '@primer/behaviors' import type {AnchorPosition, PositionSettings} from '@primer/behaviors' import {useProvidedRefOrCreate} from './useProvidedRefOrCreate' -import {useResizeObserver} from './useResizeObserver' import useLayoutEffect from '../utils/useIsomorphicLayoutEffect' export interface AnchoredPositionHookSettings extends Partial { @@ -91,14 +90,77 @@ export function useAnchoredPosition( [floatingElementRef, anchorElementRef, ...dependencies], ) + // Store updatePosition in a ref to avoid re-subscribing listeners when dependencies change. + // The ref always has the latest function, so listeners don't need updatePosition in their deps. + const updatePositionRef = React.useRef(updatePosition) + useLayoutEffect(() => { + updatePositionRef.current = updatePosition + }) + useLayoutEffect(() => { savedOnPositionChange.current = settings?.onPositionChange }, [settings?.onPositionChange]) + // Recalculate position when dependencies change useLayoutEffect(updatePosition, [updatePosition]) - useResizeObserver(updatePosition) // watches for changes in window size - useResizeObserver(updatePosition, floatingElementRef as React.RefObject) // watches for changes in floating element size + // Window resize listener for viewport changes. + // Uses updatePositionRef to avoid re-subscribing on every dependency change. + React.useEffect(() => { + const handleResize = () => updatePositionRef.current() + // eslint-disable-next-line github/prefer-observers + window.addEventListener('resize', handleResize) + return () => window.removeEventListener('resize', handleResize) + }, []) + + // Single coalesced ResizeObserver for floating element and anchor element. + // This reduces layout reads during resize events (better INP) by batching + // observations into one callback instead of triggering updatePosition 2x. + // Uses updatePositionRef to avoid re-creating observer on dependency changes. + useLayoutEffect(() => { + const floatingEl = floatingElementRef.current + const anchorEl = anchorElementRef.current + + if (typeof ResizeObserver !== 'function') { + return + } + + // First callback must be immediate - ResizeObserver fires synchronously + // on observe() and positioning must be correct before paint + let isFirstCallback = true + let pendingFrame: number | null = null + + const observer = new ResizeObserver(() => { + if (isFirstCallback) { + isFirstCallback = false + updatePositionRef.current() + return + } + + // Subsequent callbacks are throttled with rAF for better INP + if (pendingFrame === null) { + pendingFrame = requestAnimationFrame(() => { + pendingFrame = null + updatePositionRef.current() + }) + } + }) + + // Observe floating and anchor elements if available + if (floatingEl instanceof Element) { + observer.observe(floatingEl) + } + if (anchorEl instanceof Element) { + observer.observe(anchorEl) + } + + return () => { + if (pendingFrame !== null) { + cancelAnimationFrame(pendingFrame) + } + observer.disconnect() + } + }, [floatingElementRef, anchorElementRef]) return { floatingElementRef, diff --git a/packages/react/src/hooks/useOverflow.ts b/packages/react/src/hooks/useOverflow.ts index 0c394d095e5..a2a8c014612 100644 --- a/packages/react/src/hooks/useOverflow.ts +++ b/packages/react/src/hooks/useOverflow.ts @@ -8,20 +8,51 @@ export function useOverflow(ref: React.RefObject) { return } - const observer = new ResizeObserver(entries => { + // Track whether this is the first callback (fires immediately on observe()) + let isFirstCallback = true + let pendingFrame: number | null = null + let latestEntries: ResizeObserverEntry[] | null = null + + const checkOverflow = (entries: ResizeObserverEntry[]) => { for (const entry of entries) { if ( entry.target.scrollHeight > entry.target.clientHeight || entry.target.scrollWidth > entry.target.clientWidth ) { setHasOverflow(true) - break + return } } + setHasOverflow(false) + } + + const observer = new ResizeObserver(entries => { + // First callback must be immediate - ResizeObserver fires synchronously + // on observe() and consumers may depend on this timing + if (isFirstCallback) { + isFirstCallback = false + checkOverflow(entries) + return + } + + // Subsequent callbacks are throttled to reduce layout thrashing + // during rapid resize events (e.g., window drag) + latestEntries = entries + if (pendingFrame === null) { + pendingFrame = requestAnimationFrame(() => { + pendingFrame = null + if (latestEntries) { + checkOverflow(latestEntries) + } + }) + } }) observer.observe(ref.current) return () => { + if (pendingFrame !== null) { + cancelAnimationFrame(pendingFrame) + } observer.disconnect() } }, [ref]) diff --git a/packages/react/src/hooks/useResizeObserver.ts b/packages/react/src/hooks/useResizeObserver.ts index 704673af082..8f7082d9c9d 100644 --- a/packages/react/src/hooks/useResizeObserver.ts +++ b/packages/react/src/hooks/useResizeObserver.ts @@ -28,13 +28,39 @@ export function useResizeObserver( } if (typeof ResizeObserver === 'function') { + // Track whether this is the first callback (fires immediately on observe()) + let isFirstCallback = true + let pendingFrame: number | null = null + let latestEntries: ResizeObserverEntry[] | null = null + const observer = new ResizeObserver(entries => { - savedCallback.current(entries) + // First callback must be immediate - ResizeObserver fires synchronously + // on observe() and consumers may depend on this timing + if (isFirstCallback) { + isFirstCallback = false + savedCallback.current(entries) + return + } + + // Subsequent callbacks are throttled to reduce layout thrashing + // during rapid resize events (e.g., window drag) + latestEntries = entries + if (pendingFrame === null) { + pendingFrame = requestAnimationFrame(() => { + pendingFrame = null + if (latestEntries) { + savedCallback.current(latestEntries) + } + }) + } }) observer.observe(targetEl) return () => { + if (pendingFrame !== null) { + cancelAnimationFrame(pendingFrame) + } observer.disconnect() } } else { diff --git a/packages/react/src/internal/utils/hasInteractiveNodes.ts b/packages/react/src/internal/utils/hasInteractiveNodes.ts index b74c52dd9fe..00dbe65ecde 100644 --- a/packages/react/src/internal/utils/hasInteractiveNodes.ts +++ b/packages/react/src/internal/utils/hasInteractiveNodes.ts @@ -22,6 +22,9 @@ const interactiveElements = interactiveElementsSelectors.map( selector => `${selector}:not(${Object.values(nonValidSelectors).join('):not(')})`, ) +// Combined selector for fast querySelector check +const interactiveSelector = interactiveElements.join(', ') + /** * Finds interactive nodes within the passed node. * If the node itself is interactive, or children within are, it will return true. @@ -38,32 +41,30 @@ export function hasInteractiveNodes(node: HTMLElement | null, ignoreNodes?: HTML // If one does exist, we can abort early. const nodesToIgnore = ignoreNodes ? [node, ...ignoreNodes] : [node] - const interactiveNodes = findInteractiveChildNodes(node, nodesToIgnore) - return Boolean(interactiveNodes) + // Performance optimization: Use querySelectorAll with combined selector first + // This avoids recursive getComputedStyle calls for each node + const candidates = node.querySelectorAll(interactiveSelector) + for (const candidate of candidates) { + if (!nodesToIgnore.includes(candidate) && !isNonValidInteractiveNode(candidate)) { + return true + } + } + + return false } +// Cache for visibility checks to avoid repeated getComputedStyle calls during a single traversal +// Note: Only call getComputedStyle when CSS-based checks are insufficient function isNonValidInteractiveNode(node: HTMLElement) { - const nodeStyle = getComputedStyle(node) + // Fast path: Check attribute-based states first (no style recalc needed) const isNonInteractive = node.matches('[disabled], [hidden], [inert]') - const isHiddenVisually = nodeStyle.display === 'none' || nodeStyle.visibility === 'hidden' - - return isNonInteractive || isHiddenVisually -} + if (isNonInteractive) return true -function findInteractiveChildNodes(node: HTMLElement | null, ignoreNodes: HTMLElement[]) { - if (!node) return - - const ignoreSelector = ignoreNodes.find(elem => elem === node) - const isNotValidNode = isNonValidInteractiveNode(node) - - if (node.matches(interactiveElements.join(', ')) && !ignoreSelector && !isNotValidNode) { - return node - } - - for (const child of node.children) { - const interactiveNode = findInteractiveChildNodes(child as HTMLElement, ignoreNodes) + // Only call getComputedStyle if attribute checks passed + // This is necessary for display:none and visibility:hidden which aren't detectable via attributes + const nodeStyle = getComputedStyle(node) + const isHiddenVisually = nodeStyle.display === 'none' || nodeStyle.visibility === 'hidden' - if (interactiveNode) return true - } + return isHiddenVisually } diff --git a/packages/react/src/live-region/Announce.tsx b/packages/react/src/live-region/Announce.tsx index 6e439d10a9b..7d3a69fbe0f 100644 --- a/packages/react/src/live-region/Announce.tsx +++ b/packages/react/src/live-region/Announce.tsx @@ -64,12 +64,9 @@ export function Announce(props: AnnouncePr return } - const style = window.getComputedStyle(element) - if (style.display === 'none') { - return - } - - if (style.visibility === 'hidden') { + // Check if element is visible - batch these layout reads together + const {display, visibility} = window.getComputedStyle(element) + if (display === 'none' || visibility === 'hidden') { return }