Repository navigation
Editor: Improve Header layout #62636
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
Changes from all commits
4d97506
9811a80
e4720f5
ac6aca8
3399512
cb5927b
80d0531
2cf9145
276089e
04e720c
ba4e943
d4ce22b
574b8bc
884c534
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,6 +77,3 @@ | |
| } | ||
| } | ||
|
|
||
| .editor-collapsible-block-toolbar__toggle { | ||
| margin-left: 2px; // Allow focus ring to be fully visible | ||
| } | ||
|
Comment on lines
-80
to
-82
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The parent element’s style changes now ensure this isn’t necessary. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ | |
| min-width: 0; | ||
| background: $gray-100; | ||
| border-radius: $grid-unit-05; | ||
| width: min(100%, 416px); | ||
| width: min(100%, 450px); | ||
|
|
||
| &:hover { | ||
| background-color: $gray-200; | ||
|
|
@@ -25,10 +25,6 @@ | |
| background: $gray-200; | ||
| } | ||
| } | ||
|
|
||
| @include break-large() { | ||
| width: min(100%, 450px); | ||
| } | ||
|
Comment on lines
-28
to
-31
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure just why the width needed a distinct maximum at the large breakpoint. I moved the |
||
| } | ||
|
|
||
| .editor-document-bar__command { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,12 +65,12 @@ | |
| .editor-document-tools__left { | ||
| display: inline-flex; | ||
| align-items: center; | ||
| padding-left: $grid-unit-20; | ||
| gap: $grid-unit-10; | ||
|
|
||
| // Some plugins add buttons here despite best practices. | ||
| // Push them a bit rightwards to fit the top toolbar. | ||
| margin-right: $grid-unit-10; | ||
| // Some plugins add buttons here despite best practices — accommodate them with a gap. | ||
| &:not(:last-child) { | ||
| margin-inline-end: $grid-unit-10; | ||
| } | ||
|
Comment on lines
-71
to
+73
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought conditionally adding this "gap" was better than universally doing so. It saves a bit of space when plugins aren’t adding buttons there (or "Top toolbar" is not on as it also serves there). Note this change would leave no space before the collapsible block toolbar’s toggle button so that is instead handled by header styles. |
||
| } | ||
|
|
||
| .editor-document-tools .editor-document-tools__left > .editor-document-tools__inserter-toggle.has-icon { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,16 +9,16 @@ import { | |
| // Keeping an old name for backward compatibility. | ||
| const slotName = '__experimentalMainDashboardButton'; | ||
|
|
||
| export const useHasBackButton = () => { | ||
| const fills = useSlotFills( slotName ); | ||
| return Boolean( fills && fills.length ); | ||
| }; | ||
|
|
||
| const { Fill, Slot } = createSlotFill( slotName ); | ||
|
|
||
| const BackButton = Fill; | ||
| const BackButtonSlot = ( { children } ) => { | ||
| const BackButtonSlot = () => { | ||
| const fills = useSlotFills( slotName ); | ||
| const hasFills = Boolean( fills && fills.length ); | ||
|
|
||
| if ( ! hasFills ) { | ||
| return children; | ||
| } | ||
|
Comment on lines
-17
to
-21
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stokesman Is this change required? If yes, can you elaborate more on "why"? Thanks.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn’t have to remove this but it seemed redundant if we let the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the explanation, @stokesman! I guess it's okay to change the component's behavior; it's a private component, so there are no requirements for BC. |
||
|
|
||
| return ( | ||
| <Slot | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,18 @@ | ||
| .editor-header { | ||
| height: $header-height; | ||
| background: $white; | ||
| display: flex; | ||
| flex-wrap: wrap; | ||
| display: grid; | ||
| grid-auto-flow: row; | ||
| grid-template: auto / $header-height minmax(0, max-content) minmax(min-content, 1fr) $header-height; | ||
| &:has(> .editor-header__center) { | ||
| grid-template: auto / $header-height min-content 1fr min-content $header-height; | ||
| @include break-medium { | ||
| grid-template: auto / $header-height minmax(min-content, 1fr) 2fr minmax(min-content, 1fr) $header-height; | ||
| } | ||
| } | ||
| @include break-mobile { | ||
| gap: $grid-unit-20; | ||
| } | ||
| align-items: center; | ||
| // The header should never be wider than the viewport, or buttons might be hidden. Especially relevant at high zoom levels. Related to https://core.trac.wordpress.org/ticket/47603#ticket. | ||
| max-width: 100vw; | ||
|
|
@@ -15,19 +25,32 @@ | |
| } | ||
|
|
||
| .editor-header__toolbar { | ||
| grid-column: 1 / 3; | ||
| // When there is no back button or the viewport is <= mobile the margin keeps the content off | ||
| // the starting edge. | ||
| > :first-child { | ||
| margin-inline: $grid-unit-20 0; | ||
| } | ||
|
|
||
| // This is the typical case, the back button takes up the first column. | ||
| .editor-header__back-button + & { | ||
| grid-column: 2 / 3; | ||
|
|
||
| @include break-mobile { | ||
| // Clears the margin; at this breakpoint the parent’s `gap` takes its place. | ||
| > :first-child { | ||
| margin-inline: 0; | ||
| } | ||
| } | ||
| } | ||
| display: flex; | ||
| // Allow this area to shrink to fit the toolbar buttons. | ||
| flex-shrink: 8; | ||
| // Take up the space of the toolbar so it can be justified to the left side of the toolbar. | ||
| flex-grow: 3; | ||
| // Hide the overflow so flex will limit its width. Block toolbar will allow scrolling on fixed toolbar. | ||
| overflow: hidden; | ||
| // Leave enough room for the focus ring to show. | ||
| padding: 2px 0; | ||
| // Make narrowing to less than content width possible. Block toolbar will hide overflow and allow scrolling on fixed toolbar. | ||
| min-width: 0; | ||
| align-items: center; | ||
| // Allow focus ring to be fully visible on furthest right button. | ||
| @include break-medium() { | ||
| padding-right: var(--wp-admin-border-width-focus); | ||
| // Clip the box while leaving room for focus rings. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this added? It's hard to understand from this comment, it doesn't mention why there's a clip at all
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| clip-path: inset(-2px); | ||
| @include break-mobile { | ||
| clip-path: none; | ||
| } | ||
|
|
||
| .table-of-contents { | ||
|
|
@@ -37,20 +60,35 @@ | |
| display: block; | ||
| } | ||
| } | ||
|
|
||
| // Add a gap before the block toolbar or its toggle button when collapsed. | ||
| .editor-collapsible-block-toolbar { | ||
| margin-inline: $grid-unit 0; | ||
|
|
||
| &.is-collapsed ~ .editor-collapsible-block-toolbar__toggle { | ||
| margin-inline: $grid-unit 0; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| .editor-header__center { | ||
| flex-grow: 1; | ||
| grid-column: 3 / 4; | ||
| display: flex; | ||
| justify-content: center; | ||
| // Flex items will, by default, refuse to shrink below a minimum | ||
| // intrinsic width. In order to shrink this flexbox item, and | ||
| // subsequently truncate child text, we set an explicit min-width. | ||
| // See https://dev.w3.org/csswg/css-flexbox/#min-size-auto | ||
| align-items: center; | ||
| // To enable shrinking and truncating of child text, apply an explicit min-width. | ||
| min-width: 0; | ||
|
|
||
| &.is-collapsed { | ||
| display: none; | ||
| // Clip the box while leaving room for focus rings. | ||
| clip-path: inset(-2px); | ||
| // At less than mobile the header’s `gap` is zero so margins are added to create a smaller | ||
| // gap around the center’s contents. | ||
| @media (max-width: #{$break-mobile - 1}) { | ||
| > :first-child { | ||
| margin-inline-start: $grid-unit; | ||
| } | ||
| > :last-child { | ||
| margin-inline-end: $grid-unit; | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -59,6 +97,11 @@ | |
| */ | ||
|
|
||
| .editor-header__settings { | ||
| grid-column: 3 / -1; | ||
| .editor-header:has(> .editor-header__center) & { | ||
| grid-column: 4 / -1; | ||
| } | ||
| justify-self: end; | ||
| display: inline-flex; | ||
| align-items: center; | ||
| flex-wrap: nowrap; | ||
|
|
||


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.
Typo correction, the filename is correct but the component itself was typo’d.
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.
Nice catch!