ui/Dialog: update Header layout, refactor Title to use Text#77161
Merged
Conversation
ciampo
force-pushed
the
fix/dialog-header-layout
branch
from
c5d9fee to
994acfc
Compare
|
Size Change: 0 B Total Size: 7.74 MB ℹ️ View Unchanged
|
|
Flaky tests detected in a705a59. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24245773783
|
ciampo
force-pushed
the
fix/dialog-header-layout
branch
from
994acfc to
a42e1cd
Compare
ciampo
changed the title
Dialog: update Header layout to support multiple trailing elementsDialog: update Header layout, refactor Title to use Text
ciampo
force-pushed
the
fix/dialog-header-layout
branch
from
a42e1cd to
1045470
Compare
ciampo
changed the title
Dialog: update Header layout, refactor Title to use TextDialog: update Header layout, refactor Title to use Text
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
jameskoster
reviewed
Apr 10, 2026
| font-weight: var(--wpds-font-weight-medium); | ||
| line-height: var(--wpds-font-line-height-xl); | ||
| color: var(--wpds-color-fg-content-neutral); | ||
| margin-inline-end: auto; |
Contributor
There was a problem hiding this comment.
In my testing this isn't being applied for some reason 🤔
Contributor
Author
There was a problem hiding this comment.
Well spotted — this is a side-effect of the changes from #76783, where the defense styles for headings have higher specificity of the margin-inline-end rule I had originally written.
I worked around this limitation by passing the margin value via the --_gcd-heading-margin, although that means that we can't use logical properties — I used a separate :dir(rtl) rule.
cc @mirka
Member
There was a problem hiding this comment.
@ciampo Not sure how we can enforce this programmatically, but let's make sure the CSS works without the gcd stuff also. Because the assumption is that one day we're going to remove the gcd code.
So in this case I think we do want to keep the margin-inline-end: auto; rule as well.
Replace `justify-content: space-between` with `gap` + `margin-inline-end: auto` on the title, so that any number of elements after the title are properly spaced. Made-with: Cursor
Refactor Dialog.Title to render through the `Text` component with the `heading-xl` variant, matching the pattern already used by Popover.Title. This delegates typography (font-family, size, weight, line-height) to the design system's `Text` component instead of duplicating it in CSS. The `.title` CSS class is reduced to just `margin-inline-end: auto` (needed for the header flex layout), since Text now handles all typographic properties. AlertDialog's inline title also drops the `dialogStyles.title` class override, relying solely on `Text variant="heading-xl"` for consistency. Made-with: Cursor
ciampo
force-pushed
the
fix/dialog-header-layout
branch
from
1045470 to
dea117b
Compare
ciampo
force-pushed
the
fix/dialog-header-layout
branch
from
dea117b to
a705a59
Compare
jameskoster
approved these changes
Apr 10, 2026
ciampo
added a commit
that referenced
this pull request
Apr 14, 2026
The `.title` class relied solely on the `--_gcd-heading-margin` CSS variable to push trailing header elements to the end. This breaks once the global CSS defense layer is removed. Add a direct `margin-inline-end: auto` rule so the header flex layout is preserved independently of GCD styles. Follow-up to #77161, addressing review feedback from @mirka. Made-with: Cursor
ciampo
added a commit
that referenced
this pull request
Apr 14, 2026
The `.title` class relied solely on the `--_gcd-heading-margin` CSS variable to push trailing header elements to the end. This breaks once the global CSS defense layer is removed. Add a direct `margin-inline-end: auto` rule so the header flex layout is preserved independently of GCD styles. Follow-up to #77161, addressing review feedback from @mirka. Made-with: Cursor
ciampo
added a commit
that referenced
this pull request
Apr 14, 2026
The `.title` class relied solely on the `--_gcd-heading-margin` CSS variable to push trailing header elements to the end. This breaks once the global CSS defense layer is removed. Add a direct `margin-inline-end: auto` rule so the header flex layout is preserved independently of GCD styles. Follow-up to #77161, addressing review feedback from @mirka. Made-with: Cursor
ciampo
added a commit
that referenced
this pull request
Apr 14, 2026
The `.title` class relied solely on the `--_gcd-heading-margin` CSS variable to push trailing header elements to the end. This breaks once the global CSS defense layer is removed. Add a direct `margin-inline-end: auto` rule so the header flex layout is preserved independently of GCD styles. Follow-up to #77161, addressing review feedback from @mirka. Made-with: Cursor
juanfra
added a commit
that referenced
this pull request
Apr 14, 2026
The `.title` class relied solely on the `--_gcd-heading-margin` CSS variable to push trailing header elements to the end. This breaks once the global CSS defense layer is removed. Add a direct `margin-inline-end: auto` rule so the header flex layout is preserved independently of GCD styles. Follow-up to #77161, addressing review feedback from @mirka. Made-with: Cursor Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: juanfra <juanfra@git.wordpress.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Follow up to #75390
Two small
Dialogimprovements:Headerlayout to support multiple trailing elements alongside the title.Titleto useTextinternally with theheading-xlvariant, matchingPopover.Title.Why?
.headerusedjustify-content: space-between, which breaks when more than two children are present (e.g. an action button next to the close icon).Dialog.Titleduplicated typography rules in CSS instead of delegating to theTextcomponent likePopover.Titlealready does.How?
Header layout (CSS-only):
justify-content: space-betweenwithgapon.headermargin-inline-end: autoon.titleto push trailing elements to the endTitle refactor:
Dialog.Titlerenders through<Text variant="heading-xl">, same pattern asPopover.Title.titleCSS reduced tomargin-inline-end: autoonly; typography handled byTextAlertDialogdrops thedialogStyles.titleoverride, relying onText variant="heading-xl"Scope notes
Only
Dialog.Headeris affected by the layout change:AlertDialoguses a verticalStackNoticeuses CSS gridPopover/Drawerdon't have a Header subcomponentTesting Instructions
<Dialog.Title>inside<Dialog.Header>— confirm proper spacingheading-xldesign tokenTesting Instructions for Keyboard
No keyboard interaction changes.
Use of AI Tools
Made with Cursor.