diff --git a/.changeset/react-compiler-label-group.md b/.changeset/react-compiler-label-group.md new file mode 100644 index 00000000000..79fa129f270 --- /dev/null +++ b/.changeset/react-compiler-label-group.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +LabelGroup: Improve rendering performance with React Compiler support. diff --git a/packages/react/script/react-compiler.mjs b/packages/react/script/react-compiler.mjs index 95a9ee98b36..840e1825f89 100644 --- a/packages/react/script/react-compiler.mjs +++ b/packages/react/script/react-compiler.mjs @@ -15,7 +15,6 @@ const unsupportedPatterns = [ 'src/ActionMenu/ActionMenu.test.tsx', 'src/AnchoredOverlay/AnchoredOverlay.tsx', 'src/FilteredActionList/FilteredActionList.tsx', - 'src/LabelGroup/LabelGroup.tsx', 'src/Overlay/Overlay.figma.tsx', 'src/SelectPanel/SelectPanel.examples.stories.tsx', 'src/SelectPanel/SelectPanel.test.tsx', diff --git a/packages/react/src/LabelGroup/LabelGroup.test.tsx b/packages/react/src/LabelGroup/LabelGroup.test.tsx index d35c143d599..e670bcce013 100644 --- a/packages/react/src/LabelGroup/LabelGroup.test.tsx +++ b/packages/react/src/LabelGroup/LabelGroup.test.tsx @@ -171,6 +171,71 @@ describe('LabelGroup', () => { expect(fourthLabel).toBeVisible() }) + it('sizes the overlay to cover the labels through the expand button after truncation changes', async () => { + const user = userEvent.setup() + const example = (visibleChildCount: number) => { + return ( + + + + + + + + ) + } + const {getByRole, rerender} = render(example(1)) + + for (const visibleChildCount of [1, 2]) { + rerender(example(visibleChildCount)) + const expandButton = getByRole('button', { + name: `Show +${3 - visibleChildCount} more`, + }) + const expectedWidth = + expandButton.getBoundingClientRect().right - getByRole('list').getBoundingClientRect().left + 8 + + await user.click(expandButton) + + const overlayContainer = getByRole('dialog').querySelector(`.${classes.OverlayContainer}`) + expect(overlayContainer).not.toBeNull() + expect(Number.parseFloat(overlayContainer?.style.width ?? '')).toBeCloseTo(expectedWidth) + + await user.click( + getByRole('button', { + name: 'Close', + }), + ) + await waitFor(() => { + expect(expandButton).toHaveFocus() + }) + } + }) + + it('focuses the first previously hidden control when expanded inline', async () => { + const user = userEvent.setup() + const {getByRole} = render( + + + + + + + , + ) + + await user.click( + getByRole('button', { + name: 'Show +2 more', + }), + ) + + expect( + getByRole('button', { + name: 'Two', + }), + ).toHaveFocus() + }) + it('should focus the collapse button when expanded, and the expand button when collapsed', async () => { const user = userEvent.setup() const {getByText} = render( diff --git a/packages/react/src/LabelGroup/LabelGroup.tsx b/packages/react/src/LabelGroup/LabelGroup.tsx index 3fb0727b03f..1e84c155329 100644 --- a/packages/react/src/LabelGroup/LabelGroup.tsx +++ b/packages/react/src/LabelGroup/LabelGroup.tsx @@ -5,6 +5,7 @@ import VisuallyHidden from '../_VisuallyHidden' import {AnchoredOverlay} from '../AnchoredOverlay' import {Button, IconButton} from '../Button' import {clsx} from 'clsx' +import useLayoutEffect from '../utils/useIsomorphicLayoutEffect' import classes from './LabelGroup.module.css' export type LabelGroupProps = { @@ -16,17 +17,10 @@ export type LabelGroupProps = { visibleChildCount?: 'auto' | number className?: string } -// Calculates the width of the overlay to cover the labels/tokens and the expand button. -const getOverlayWidth = ( - buttonClientRect: DOMRect, - containerRef: React.RefObject, - overlayPaddingPx: number, -) => overlayPaddingPx + buttonClientRect.right - (containerRef.current?.getBoundingClientRect().left || 0) - const InlineToggle: React.FC<{ collapseButtonRef: React.RefObject collapseInlineExpandedChildren: () => void - expandButtonRef: React.RefCallback + expandButtonRef: React.RefObject hiddenItemIds: string[] isOverflowShown: boolean showAllTokensInline: () => void @@ -53,7 +47,7 @@ const InlineToggle: React.FC<{ const OverlayToggle: React.FC< React.PropsWithChildren<{ closeOverflowOverlay: () => void - expandButtonRef: React.RefCallback + expandButtonRef: React.RefObject hiddenItemIds: string[] isOverflowShown: boolean openOverflowOverlay: () => void @@ -81,8 +75,7 @@ const OverlayToggle: React.FC< height="auto" align="start" side="inside-right" - // expandButtonRef satisfies React.RefObject because we manually set `.current` in the `useCallback` above - anchorRef={expandButtonRef as unknown as React.RefObject} + anchorRef={expandButtonRef} anchorOffset={overlayPaddingPx * -1} alignmentOffset={overlayPaddingPx * -1} renderAnchor={props => ( @@ -116,51 +109,49 @@ const LabelGroup: React.FC> = ({ className, }) => { const containerRef = React.useRef(null) + const expandButtonRef = React.useRef(null) const collapseButtonRef = React.useRef(null) const firstHiddenIndexRef = React.useRef(undefined) const [visibilityMap, setVisibilityMap] = React.useState>({}) const [isOverflowShown, setIsOverflowShown] = React.useState(false) - const [buttonClientRect, setButtonClientRect] = React.useState({ - width: 0, - right: 0, - height: 0, - x: 0, - y: 0, - top: 0, - left: 0, - bottom: 0, - toJSON: () => undefined, - }) + const [buttonWidth, setButtonWidth] = React.useState(0) + const [overlayWidth, setOverlayWidth] = React.useState(undefined) const overlayPaddingPx = 8 // var(--base-size-8), hardcoded to do some math const hiddenItemIds = Object.keys(visibilityMap).filter(key => !visibilityMap[key]) - // `overlayWidth` is only needed when we render an overlay - // if we don't use an overlay, we can skip the width calculation - // and save on reflows caused by measuring DOM nodes. - const overlayWidth = - hiddenItemIds.length && overflowStyle === 'overlay' - ? // eslint-disable-next-line react-hooks/refs - getOverlayWidth(buttonClientRect, containerRef, overlayPaddingPx) - : undefined - - const expandButtonRef: React.RefCallback = React.useCallback( - // eslint-disable-next-line react-hooks/immutability - node => { - if (node !== null) { - const nodeClientRect = node.getBoundingClientRect() + // Measure layout changes after commit rather than reading refs during render. + // Equality checks keep measurement updates from causing a loop. + useLayoutEffect(() => { + const button = expandButtonRef.current + if (!visibleChildCount || !button) { + return + } - if (nodeClientRect.width !== buttonClientRect.width || nodeClientRect.right !== buttonClientRect.right) { - setButtonClientRect(nodeClientRect) - } + const buttonClientRect = button.getBoundingClientRect() + if (buttonClientRect.width !== buttonWidth) { + setButtonWidth(buttonClientRect.width) + } - // @ts-ignore you can set `.current` on ref objects or ref callbacks in React - // eslint-disable-next-line react-hooks/immutability - expandButtonRef.current = node + // Only measure the container when an overlay needs to cover the labels. + if (hiddenItemIds.length && overflowStyle === 'overlay') { + const width = + overlayPaddingPx + buttonClientRect.right - (containerRef.current?.getBoundingClientRect().left || 0) + if (width !== overlayWidth) { + setOverlayWidth(width) } - }, - [buttonClientRect], - ) + } + }, [ + Component, + buttonWidth, + children, + className, + hiddenItemIds, + isOverflowShown, + overflowStyle, + overlayWidth, + visibleChildCount, + ]) // Sets the visibility map to hide children after the given index. const hideChildrenAfterIndex = React.useCallback((truncateAfter: number) => { @@ -193,7 +184,6 @@ const LabelGroup: React.FC> = ({ // list in an overlay. // TODO: get rid of this hack setTimeout(() => { - // @ts-ignore you can set `.current` on ref objects or ref callbacks in React expandButtonRef.current?.focus() }, 10) }, [expandButtonRef, hideChildrenAfterIndex, visibleChildCount]) @@ -213,7 +203,7 @@ const LabelGroup: React.FC> = ({ if (visibleChildCount === 'auto') { // Instantiates the IntersectionObserver to track when children fit in the container. const observer = new IntersectionObserver( - (entries: IntersectionObserverEntry[]) => { + (entries: Array) => { const updatedEntries: Record = {} for (const entry of entries) { @@ -232,7 +222,7 @@ const LabelGroup: React.FC> = ({ }, { root: containerRef.current, - rootMargin: `0px -${buttonClientRect.width}px 0px 0px`, + rootMargin: `0px -${buttonWidth}px 0px 0px`, threshold: 1, }, ) @@ -250,7 +240,7 @@ const LabelGroup: React.FC> = ({ // eslint-disable-next-line react-hooks/set-state-in-effect hideChildrenAfterIndex(visibleChildCount) } - }, [buttonClientRect, visibleChildCount, hideChildrenAfterIndex, isOverflowShown]) + }, [buttonWidth, visibleChildCount, hideChildrenAfterIndex, isOverflowShown]) // Updates the index of the first hidden child. // We need to keep track of this so we can focus the first hidden child when the overflow is shown inline.