Refactor Admin UI / Breadcrumbs to use DS components and design tokens#77012
Open
jameskoster wants to merge 9 commits intotrunkfrom
Open
Refactor Admin UI / Breadcrumbs to use DS components and design tokens#77012jameskoster wants to merge 9 commits intotrunkfrom
jameskoster wants to merge 9 commits intotrunkfrom
Conversation
jameskoster
added
Design System
jameskoster
commented
Apr 2, 2026
| li:not(:last-child)::after { | ||
| content: "/"; | ||
| margin: 0 var(--wpds-dimension-gap-sm); | ||
| color: var(--wpds-color-stroke-surface-neutral); |
Contributor
Author
There was a problem hiding this comment.
I figure this is okay as the separator is a decorative element and the list structure conveys meaning to assistive tech. I appreciate it's a bit of a gray area though. Thoughts welcome.
Contributor
There was a problem hiding this comment.
Does this affect hover styles and general click area of the item, though?
|
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
commented
Apr 2, 2026
| const ALLOWLIST = { | ||
| '@wordpress/ui': { | ||
| allowed: [ 'Badge', 'Stack' ], | ||
| allowed: [ 'Badge', 'Link', 'Stack', 'Text' ], |
Contributor
Author
There was a problem hiding this comment.
If this needs to be handled separately I understand :)
Contributor
There was a problem hiding this comment.
Just flagging that Text is now allowed, and I guess Link may soon allowed too?
|
Size Change: +145 B (0%) Total Size: 7.74 MB 📦 View Changed
ℹ️ View Unchanged
|
|
Flaky tests detected in 4710a2f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24247473882
|
aduth
reviewed
Apr 3, 2026
Comment on lines
3
to
4
| font-size: var(--wpds-font-size-lg); | ||
| font-weight: var(--wpds-font-weight-medium); | ||
| line-height: var(--wpds-font-line-height-lg); |
Member
There was a problem hiding this comment.
My assumption would be we'd not want to be overriding any of these anymore if we're using Text component? Do font-size and line-height not work for what we want?
Contributor
Author
There was a problem hiding this comment.
Text is only used for the last item in the trail. These styles are to ensure the Links size and line-height match. Though I suppose we could replace nav with a Text instance that renders as nav, then remove these styles. I'll try that :)
Member
There was a problem hiding this comment.
Yep, these styles are here from earlier work to make the title + breadcrumbs consistent with CSS, since Text wasn't ready to use yet. Now that Text is safe, we should update Text across the admin-ui to ensure consistency.
Worth updating also the title to use Text either in this PR or separately; happy to follow-up with separate PR if you'd prefer.
aduth
reviewed
Apr 8, 2026
Member
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
This looks like it's in pretty good shape, but needs a rebase to remove the pieces that have been addressed separately for allowlisting Text usage and handling jsx-a11y/heading-has-content.
| </Link> | ||
| ) : ( | ||
| <Heading level={ 1 } truncate> | ||
| /* eslint-disable jsx-a11y/heading-has-content */ |
Member
There was a problem hiding this comment.
We can remove this after rebase, since this was addressed via #77073.
| li:not(:last-child)::after { | ||
| content: "/"; | ||
| margin: 0 variables.$grid-unit-10; | ||
| // @TODO: Remove truncation styles once `Text` supports truncation natively. |
Member
There was a problem hiding this comment.
I can help write an issue for this, but what do we think this'll look like? A truncate prop like what we had with the @wordpress/components's Text component?
Contributor
Author
There was a problem hiding this comment.
Yes I think so, potentially with a numberOfLines (or equivalent) option as well.
Refactor breadcrumb links to compose the UI Link with the router Link via the render prop, adopting design-system styling. Replace the base-styles spacing variable with a design token and add Link to the eslint allowlist for @wordpress/ui. Made-with: Cursor
Made-with: Cursor
Replace HStack and Heading from @wordpress/components with Stack and Text from @wordpress/ui. Add Text to the eslint allowlist for @wordpress/ui. Made-with: Cursor
Made-with: Cursor
Use an outer Text with body-lg on the nav element so links and separators inherit typography via CSS. The inner Text with heading-lg on the h1 overrides for the current item. Removes manual font declarations from the stylesheet. Made-with: Cursor
jameskoster
force-pushed
the
update/admin-ui-breadcrumbs
branch
from
b1af8da to
c950341
Compare
No longer needed after #77073. Made-with: Cursor
ciampo
reviewed
Apr 10, 2026
| as="ul" | ||
| <Text | ||
| variant="body-lg" | ||
| render={ <nav /> } |
Contributor
There was a problem hiding this comment.
While this technically works, it feels a bit unusual. My preference would be to apply Text where needed, ie. on the single Link instances eg
<Text
variant="body-lg"
render={
<Link
tone="neutral"
render={ <RouterLink to={ item.to } /> }
/>
}
>
{ item.label }
</Text>
Contributor
Author
There was a problem hiding this comment.
Hah, I had the same thought but Cursor convinced me:
- Less repetition — one Text instance vs. one per item.
- No extra DOM nodes — approach B adds a wrapper around every link, which adds noise and could affect inline layout.
I guess these are very minor points though, so happy to go this way if y'all prefer.
Contributor
There was a problem hiding this comment.
I still lean towards my suggested approach (which was your initial instinct) and push back to cursor:
- re. repetition, this code mostly runs in a loop anyway, and could be abstracted to an internal
BreadcrumbsItemcomponent anyway - re. extra DOM nodes — I'm not sure Opus is right here. I'd expect only a single dom node anyway
| overflow: hidden; | ||
| text-overflow: ellipsis; | ||
| white-space: nowrap; | ||
| margin: 0; |
Contributor
There was a problem hiding this comment.
If we use Text as for my previous suggestion, we may not need margin: 0
Wrap each Link in Text with body-lg via nested render props (Text → Link → RouterLink) rather than using an outer Text on the nav element. Remove the h1 margin override. Made-with: Cursor
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
5 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
Refactors the Breadcrumbs component to use
Link,Stack, andTextfrom@wordpress/uiinstead ofHStack,Heading, and the unstyled routerLink. Replaces the@wordpress/base-stylesspacing variable with a design token and addsLinkandTextto the@wordpress/uiESLint allowlist. Makes some small style adjustments.Why
The Breadcrumbs component was relying on experimental components from
@wordpress/components(__experimentalHStack,__experimentalHeading) and an unstyled router link. Migrating to@wordpress/uicomponents aligns breadcrumbs with the design system, giving links consistent interactive styling via theneutraltone and typography viaTextvariants. It also resolves a longstanding TODO in the stylesheet about adopting theTextcomponent.How
Link(for styling) with the routerLink(for navigation) using therenderprop. Usestone="neutral"for breadcrumb-appropriate link styling.HStackwithStack direction="row", rendered as a<ul>via therenderprop.HeadingwithText variant="heading-lg", rendered as an<h1>via therenderprop. Truncation is handled via a CSS class untilTextsupports it natively.@wordpress/base-stylesimport andh1override. Replaces the Sass spacing variable with--wpds-dimension-gap-sm. Uses a subdued color for the decorative "/" separator.LinkandTextto theuse-recommended-componentsallowlist for@wordpress/ui, with corresponding test updates.Before
After
Use of AI Tools
Created with Cursor & Claude Opus 4.6