Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 677
Graduate primer_react_css_has_selector_perf feature flag#7633
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.
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
9daa85ce13237fe0b254294a5e2fe746d6914e0145045afe9a8f015624d86d815cde0bb561532c4159aaFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@primer/react': patch | ||
| --- | ||
| Graduate `primer_react_css_has_selector_perf` feature flag: the CSS `:has()` performance optimization (`body[data-dialog-scroll-disabled]`) is now the default behavior for Dialog scroll disabling |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -20,6 +20,13 @@ import {useResizeObserver} from '../hooks/useResizeObserver' | ||
| /* Dialog Version 2 */ | ||
| /** | ||
| * Ref count for data-dialog-scroll-disabled attribute management. | ||
| * Tracks how many dialogs are currently open to know when to remove the attribute. | ||
| * This is client-only: it is only accessed inside useEffect, which never runs on the server. | ||
| */ | ||
| let dialogScrollDisabledCount = 0 | ||
| /** | ||
| * Props that characterize a button to be rendered into the footer of | ||
| * a Dialog. | ||
| @@ -271,7 +278,6 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP | ||
| const autoFocusedFooterButtonRef = useRef<HTMLButtonElement>(null) | ||
| for (const footerButton of footerButtons) { | ||
| if (footerButton.autoFocus) { | ||
| // eslint-disable-next-line react-hooks/immutability | ||
| footerButton.ref = autoFocusedFooterButtonRef | ||
| } | ||
| } | ||
| @@ -313,29 +319,16 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP | ||
| React.useEffect(() => { | ||
| const scrollbarWidth = window.innerWidth - document.body.clientWidth | ||
| const dialog = dialogRef.current | ||
| const usePerfOptimization = document.body.hasAttribute('data-dialog-scroll-optimized') | ||
| // Add DisableScroll class to this dialog (for legacy :has() selector path) | ||
| dialog?.classList.add(classes.DisableScroll) | ||
| dialogScrollDisabledCount++ | ||
| document.body.style.setProperty('--prc-dialog-scrollgutter', `${scrollbarWidth}px`) | ||
| if (usePerfOptimization) { | ||
| // Optimized path: set attribute on body for direct CSS targeting | ||
| document.body.setAttribute('data-dialog-scroll-disabled', '') | ||
| } | ||
| // Legacy path: no action needed - CSS :has(.Dialog.DisableScroll) handles it | ||
| document.body.setAttribute('data-dialog-scroll-disabled', '') | ||
Comment on lines
320
to
326
CopilotAI | ||
| return () => { | ||
| dialog?.classList.remove(classes.DisableScroll) | ||
| const remainingDialogs = document.querySelectorAll(`.${classes.DisableScroll}`) | ||
| if (remainingDialogs.length === 0) { | ||
| dialogScrollDisabledCount-- | ||
| if (dialogScrollDisabledCount === 0) { | ||
| document.body.style.removeProperty('--prc-dialog-scrollgutter') | ||
| if (usePerfOptimization) { | ||
| document.body.removeAttribute('data-dialog-scroll-disabled') | ||
| } | ||
| document.body.removeAttribute('data-dialog-scroll-disabled') | ||
| } | ||
| } | ||
| }, []) | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
CopilotAIMar 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
footerButtonsloop still mutatesfooterButton.ref, but theeslint-disable-next-line react-hooks/immutabilitywas removed. If the repo’s immutability lint rule still applies here (it’s used elsewhere), this will fail linting. Either restore the targeted eslint-disable for this assignment or refactor to avoid mutating thefooterButtonsobjects (e.g., derive a new array/object for rendering).