perf(Dialog): replace :has(.Footer) with data-has-footer attribute by hectahertz · Pull Request #7599 · primer/react

hectahertz · 2026-02-26T12:11:35Z

Closes #

Replaces .Dialog:has(.Footer) with .Dialog[data-has-footer] for the scrollable body border detection rule.

Dialog already resolves whether a footer exists in JS via useSlots + renderFooter, but wasn't exposing it as a data attribute. This PR adds data-has-footer on the dialog div and swaps the :has() selector for an attribute check.

The more expensive scroll-lock :has() selector (body:has(.Dialog.DisableScroll)) is already guarded behind a feature flag (primer_react_css_has_selector_perf) and gated with :not([data-dialog-scroll-optimized]), so it's not addressed here.

Other open perf PRs: #7598, #7597, #7554, #7550, #7549, #7545, #7544

Changelog

New

N/A

Changed

Dialog CSS now uses [data-has-footer] attribute selector instead of :has(.Footer) for footer border detection

Removed

N/A

Rollout strategy

Patch release
Minor release
Major release; if selected, include a written rollout or migration plan
None; if selected, include a brief description as to why

Testing & Reviewing

One new data-has-footer attribute on the dialog div, one CSS selector change. The animation-timeline: scroll(self) border logic is unchanged.

Open Storybook, navigate to Dialog stories
Open a dialog with footer buttons, verify the scroll border appears between body and footer when content overflows
Open a dialog without footer, verify no border artifact

Merge checklist

Added/updated tests
Added/updated documentation
Added/updated previews (Storybook)
Changes are SSR compatible
Tested in Chrome
Tested in Firefox
Tested in Safari
Tested in Edge
(GitHub staff only) Integration tests pass at github/github-ui (Learn more about how to run integration tests)

changeset-bot · 2026-02-26T12:11:39Z

🦋 Changeset detected

Latest commit: 2313366

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2026-02-26T12:12:11Z

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the integration-tests: skipped manually label to skip these checks.

Copilot

Pull request overview

This PR improves Dialog CSS performance by avoiding a parent :has() selector for footer detection and instead exposing footer presence via a data-has-footer attribute on the Dialog root.

Changes:

Add data-has-footer to the Dialog root element when a footer is rendered.
Replace .Dialog:has(.Footer) with .Dialog[data-has-footer] in Dialog CSS for the scroll border rule.
Add a changeset documenting the patch change.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
packages/react/src/Dialog/Dialog.tsx	Adds `data-has-footer` attribute derived from the computed footer node.
packages/react/src/Dialog/Dialog.module.css	Updates footer border detection selector to use `[data-has-footer]` instead of `:has(.Footer)`.
.changeset/perf-dialog-css-has-selector.md	Adds a patch changeset entry describing the selector/attribute change.

Copilot · 2026-02-26T12:15:19Z

packages/react/src/Dialog/Dialog.tsx

            {...positionDataAttributes}
            data-width={width}
            data-height={height}
+            data-has-footer={footer ? '' : undefined}


data-has-footer is derived from footer ? ... which uses a truthiness check. Since footer is a ReactNode, valid values like 0 or '' would incorrectly omit the attribute. Prefer a nullish check (e.g. set the attribute when footer != null) to match “footer rendered” semantics more reliably.

Suggested change

data-has-footer={footer ? '' : undefined}

data-has-footer={footer != null ? '' : undefined}

Copilot · 2026-02-26T12:15:19Z

packages/react/src/Dialog/Dialog.tsx

            {...positionDataAttributes}
            data-width={width}
            data-height={height}
+            data-has-footer={footer ? '' : undefined}


A new DOM contract is introduced (data-has-footer) and drives styling, but there are no unit tests asserting the attribute is present when a footer is rendered and absent when renderFooter/footer slot returns null. Adding a couple of assertions in the existing Dialog test suite would help prevent regressions.

primer-integration · 2026-03-02T15:23:34Z

👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/14866

primer-integration · 2026-03-02T15:34:30Z

Integration test results from github/github-ui:

CI Passed
VRT Passed
Projects Passed

All checks passed!

francinelucca

mostly LGTM, just was wondering about the TextInput file

francinelucca · 2026-03-03T01:39:11Z

packages/react/src/TextInput/TextInput.tsx

    const [isOverLimit, setIsOverLimit] = useState<boolean>(false)
    const [screenReaderMessage, setScreenReaderMessage] = useState<string>('')
    const characterCounterRef = useRef<CharacterCounter | null>(null)
+    const lastCountedLengthRef = useRef<number | null>(null)


what are these changes? 😅 seems unrelated to the Dialog stuff? if so can we separate into a different pr?

hectahertz · 2026-03-04T08:09:07Z

Good catch! Removed those, they already shipped in #7600.

hectahertz added 2 commits February 26, 2026 13:10

perf(Dialog): replace :has(.Footer) with data-has-footer attribute

f847580

chore: changeset

510e557

Copilot AI review requested due to automatic review settings February 26, 2026 12:11

hectahertz requested a review from a team as a code owner February 26, 2026 12:11

hectahertz requested a review from liuliu-dev February 26, 2026 12:11

Copilot started reviewing on behalf of hectahertz February 26, 2026 12:12 View session

github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Feb 26, 2026

Copilot AI reviewed Feb 26, 2026

View reviewed changes

hectahertz deployed to github-pages February 26, 2026 12:21 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-7599 February 26, 2026 12:22 Inactive

perf(TextInput): skip redundant character counter updates

b0f48dc

hectahertz mentioned this pull request Feb 26, 2026

perf(TextInput): skip redundant character counter updates #7600

Open

13 tasks

hectahertz deployed to github-pages February 26, 2026 12:51 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-7599 February 26, 2026 12:51 Inactive

hectahertz enabled auto-merge February 27, 2026 12:12

hectahertz requested a review from francinelucca March 2, 2026 14:43

francinelucca requested changes Mar 3, 2026

View reviewed changes

fix(Dialog): remove unrelated TextInput changes

977d2e1

hectahertz deployed to github-pages March 4, 2026 08:19 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-7599 March 4, 2026 08:19 Inactive

fix(Dialog): use nullish check for data-has-footer, add tests

2313366

hectahertz deployed to github-pages March 4, 2026 08:42 — with GitHub Actions View deployment

github-actions bot deployed to storybook-preview-7599 March 4, 2026 08:42 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf(Dialog): replace :has(.Footer) with data-has-footer attribute#7599

perf(Dialog): replace :has(.Footer) with data-has-footer attribute#7599
hectahertz wants to merge 5 commits intomainfrom
hectahertz/perf-dialog-css-has-selector

hectahertz commented Feb 26, 2026 •

edited

Loading

Uh oh!

changeset-bot bot commented Feb 26, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Feb 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

primer-integration bot commented Mar 2, 2026

Uh oh!

primer-integration bot commented Mar 2, 2026

Uh oh!

francinelucca left a comment

Uh oh!

francinelucca Mar 3, 2026

Uh oh!

hectahertz commented Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	data-has-footer={footer ? '' : undefined}
	data-has-footer={footer != null ? '' : undefined}

Conversation

hectahertz commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog

New

Changed

Removed

Rollout strategy

Testing & Reviewing

Merge checklist

Uh oh!

changeset-bot bot commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Feb 26, 2026

⚠️ Action required

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

primer-integration bot commented Mar 2, 2026

Uh oh!

primer-integration bot commented Mar 2, 2026

Uh oh!

francinelucca left a comment

Choose a reason for hiding this comment

Uh oh!

francinelucca Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

hectahertz commented Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hectahertz commented Feb 26, 2026 •

edited

Loading

changeset-bot bot commented Feb 26, 2026 •

edited

Loading