Skip to content
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.

Sign up for GitHub

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

Change header height from 60px to 64px #60729

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from
Open

Change header height from 60px to 64px #60729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
95d9afe
Change header height from 60px to 64px
t-hamano Oct 27, 2024
1bf3037
Improve comments for vertical block mover
t-hamano Oct 29, 2024
f3e576d
Add comment for Modal style
t-hamano Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/base-styles/_variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ $button-size: 36px;
$button-size-next-default-40px: 40px; // transitionary variable for next default button size
$button-size-small: 24px;
$button-size-compact: 32px;
$header-height: 60px;
$header-height: 64px;
$panel-header-height: $grid-unit-60;
$nav-sidebar-width: 300px;
$admin-bar-height: 32px;
Expand Down
5 changes: 3 additions & 2 deletions packages/block-editor/src/components/grid/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,12 @@

.editor-collapsible-block-toolbar {
.block-editor-grid-item-mover__move-vertical-button-container {
// Move up a little to prevent the toolbar shift when focus is on the vertical movers.
// Reduce height and move up a little to prevent the toolbar
// shift when focus is on the vertical movers.
@include break-small() {
height: $grid-unit-50;
position: relative;
top: -5px; // Should be -4px, but that causes scrolling when focus lands on the movers, in a 60px header.
top: #{$grid-unit-05 * -1};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we retain the comment, so that folks know the reason for the -1px ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made it even clearer by 1bf3037 what the intent of these styles is, including the top property.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can use perhaps $border-width instead of 1px, that might imply the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$border-width instead of 1px

Here 1px is not used. #{$grid-unit-05 * -1} becomes -4px. Am I missing something?

}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/inserter/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ $block-inserter-tabs-height: 44px;
.block-editor-block-patterns-explorer {
&__sidebar {
position: absolute;
top: $header-height + $grid-unit-15;
top: $header-height + $grid-unit-10;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required in the Pattern Inserter modal to make the sidebar exactly adjacent to the modal header.

patterns-sidebar

left: 0;
bottom: 0;
width: $sidebar-width;
Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/modal/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@
flex-direction: row;
justify-content: space-between;
align-items: center;
height: $header-height + $grid-unit-15;
height: $header-height + $grid-unit-10; // 72px
width: 100%;
z-index: z-index(".components-modal__header");
position: absolute;
Expand Down Expand Up @@ -173,7 +173,7 @@
// Modal contents.
.components-modal__content {
flex: 1;
margin-top: $header-height + $grid-unit-15;
margin-top: $header-height + $grid-unit-10;
// Small top padding required to avoid cutting off the visible outline when the first child element is focusable.
padding: $grid-unit-05 $grid-unit-40 $grid-unit-40;
overflow: auto;
Expand Down
4 changes: 2 additions & 2 deletions packages/edit-site/src/components/editor/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@
position: absolute;
top: 0;
left: 0;
width: 60px;
height: 60px;
width: $header-height;
height: $header-height;
display: flex;
align-items: center;
justify-content: center;
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-widgets/src/components/error-boundary/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
margin: auto;
max-width: 780px;
padding: 20px;
margin-top: 60px;
margin-top: $header-height;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the top margin of the content that will be displayed when a fatal error occurs in the block editor. This value was originally hardcoded, but it should be expected to match the height of the header.

error-boundary

box-shadow: $elevation-large;
}
2 changes: 1 addition & 1 deletion packages/edit-widgets/src/components/header/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
.block-editor-block-toolbar {
height: 100%;
// Push down so that buttons are centered vertically.
// It should be 14px (60px header height - 32px compact button height = 28 / 2),
// It should be 16px (64px header height - 32px compact button height = 32 / 2),
// but there is a -1px top-margin down the stack that affects this.
padding-top: math.div($header-height - $button-size-compact, 2) + 1;

Expand Down
7 changes: 4 additions & 3 deletions packages/editor/src/components/collapsible-block-toolbar/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
.block-editor-block-toolbar {
height: 100%;
// Push down so that buttons are centered vertically.
// It should be 14px (60px header height - 32px compact button height = 28 / 2),
// It should be 16px (64px header height - 32px compact button height = 32 / 2),
// but there is a -1px top-margin down the stack that affects this.
padding-top: math.div($header-height - $button-size-compact, 2) + 1;

Expand Down Expand Up @@ -62,12 +62,13 @@
overflow: visible;
}

// Move up a little to prevent the toolbar shift when focus is on the vertical movers.
// Reduce height and move up a little to prevent the toolbar
// shift when focus is on the vertical movers.
@include break-small() {
&:not(.is-horizontal) .block-editor-block-mover__move-button-container {
height: $grid-unit-50;
position: relative;
top: -5px; // Should be -4px, but that causes scrolling when focus lands on the movers, in a 60px header.
top: #{$grid-unit-05 * -1};
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/editor/src/components/error-boundary/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
margin: auto;
max-width: 780px;
padding: 20px;
margin-top: 60px;
margin-top: $header-height;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as this comment

box-shadow: $elevation-large;
}
Loading