-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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 | ||
|
|
||
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!