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

Jump to bottom

Add a generic style to the toolbar buttons #21252

Merged
merged 6 commits into from
Apr 2, 2020
Merged

Add a generic style to the toolbar buttons #21252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fb00904
Add a generic style to the toolbar buttons
youknowriad Mar 30, 2020
055e32b
more consistent toolbars
youknowriad Mar 30, 2020
5fd6b05
Opinionated styles for all toolbars
youknowriad Mar 30, 2020
4fee8f1
Fix breadcrumb
youknowriad Mar 30, 2020
0f4718b
Fix styles and round corners
youknowriad Mar 31, 2020
9642fb3
Fix unit tests
youknowriad Mar 31, 2020
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
20 changes: 9 additions & 11 deletions packages/block-editor/src/components/block-list/breadcrumb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* WordPress dependencies
*/
import { Toolbar, Button } from '@wordpress/components';
import { Button } from '@wordpress/components';
import { useSelect, useDispatch } from '@wordpress/data';
import { useEffect, useRef } from '@wordpress/element';
import { BACKSPACE, DELETE } from '@wordpress/keycodes';
Expand Down Expand Up @@ -75,16 +75,14 @@ function BlockBreadcrumb( {

return (
<div className="block-editor-block-list__breadcrumb" { ...props }>
<Toolbar>
<Button
ref={ ref }
onClick={ () => setNavigationMode( false ) }
onKeyDown={ onKeyDown }
label={ label }
>
<BlockTitle clientId={ clientId } />
</Button>
</Toolbar>
<Button
ref={ ref }
onClick={ () => setNavigationMode( false ) }
onKeyDown={ onKeyDown }
label={ label }
>
<BlockTitle clientId={ clientId } />
</Button>
</div>
);
}
Expand Down
57 changes: 17 additions & 40 deletions packages/block-editor/src/components/block-list/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,21 +477,6 @@
padding-left: $block-toolbar-height; // Provide space for the mover control on full-wide items.
}

.edit-post-header-toolbar__block-toolbar,
.block-editor-block-contextual-toolbar {
// Adapt the height of the toolbar items.
.components-toolbar {
height: $block-toolbar-height;
background: none;
}

// Adapt the height of all toolbar buttons.
.components-button {
height: $block-toolbar-height;
}
}


/**
* Block Toolbar when contextual.
*/
Expand All @@ -516,36 +501,28 @@
display: block;
z-index: z-index(".block-editor-block-list__breadcrumb");

.components-toolbar {
display: flex;
border: none;
background: none;

// The button here has a special style to appear as a toolbar.
.components-button {
font-size: $default-font-size;
height: $block-toolbar-height - $border-width - $border-width;
padding: $grid-unit-15 $grid-unit-20;
// The button here has a special style to appear as a toolbar.
.components-button {
font-size: $default-font-size;
height: $block-toolbar-height - $border-width - $border-width;
padding: $grid-unit-15 $grid-unit-20;

// Position this to align with the block toolbar.
position: relative;
top: -$border-width;
// Position this to align with the block toolbar.
position: relative;
top: -$border-width;

// Block UI appearance.
box-shadow: 0 0 0 $border-width $dark-gray-primary;
border-radius: $radius-block-ui - $border-width; // Border is outset, so so subtract the width to achieve correct radius.
background-color: $white;
// Block UI appearance.
box-shadow: 0 0 0 $border-width $dark-gray-primary;
border-radius: $radius-block-ui - $border-width; // Border is outset, so so subtract the width to achieve correct radius.
background-color: $white;

// Indent to align with block toolbar.
margin-left: $block-toolbar-height + $border-width;
// Indent to align with block toolbar.
margin-left: $block-toolbar-height + $border-width;

// When button is focused, it receives a box-shadow instead of the border.
&:focus {
box-shadow: 0 0 0 $border-width-focus $theme-color;
}
// When button is focused, it receives a box-shadow instead of the border.
&:focus {
box-shadow: 0 0 0 $border-width-focus $theme-color;
}

// @todo, it should have the block type icon here.
}
}

Expand Down
4 changes: 4 additions & 0 deletions packages/block-editor/src/components/block-mover/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
flex-direction: row;
}

.block-editor-block-mover__control {
height: $block-toolbar-height/2;
}

// Position the icons correctly.
.components-toolbar .block-editor-block-mover__control-up {
svg {
Expand Down
129 changes: 5 additions & 124 deletions packages/block-editor/src/components/block-toolbar/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,130 +44,11 @@

.block-editor-block-toolbar,
.block-editor-format-toolbar {
// Toolbar buttons.
.components-button {
position: relative;

// Give all buttons extra padding to fit text.
padding-left: $grid-unit-20;
padding-right: $grid-unit-20;

// Don't show the focus inherited by the Button component.
&:focus:enabled {
box-shadow: none;
outline: none;
}

// Focus and toggle pseudo elements.
&::before {
content: "";
position: absolute;
display: block;
border-radius: $radius-block-ui;
height: 32px;
min-width: 32px;

// Position the focus rectangle.
left: $grid-unit-10;
right: $grid-unit-10;
}

svg {
position: relative;

// Center the icon inside.
margin-left: auto;
margin-right: auto;
}

// Toggled style.
&.is-pressed {
color: $white;

&::before {
background: $dark-gray-primary;
}
}

// Focus style.
&:focus::before {
@include block-toolbar-button-style__focus();
}

// Ensure the icon buttons remain square.
&.has-icon {
// Reduce the default padding when a button only has an icon.
padding-left: $grid-unit-10;
padding-right: $grid-unit-10;

min-width: $block-toolbar-height;
justify-content: center;
}

// @todo: We should extract the tabs styles to the tabs component itself
&.components-tab-button {
font-weight: 500;

span {
display: inline-block;
padding-left: 0;
padding-right: 0;
position: relative;
}
}
}

// Size multiple sequential buttons to be optically balanced.
// Icons are 36px, as set by a 24px icon and 12px padding.
.components-toolbar div > .components-button.has-icon {
min-width: $block-toolbar-height - $grid-unit-15;
padding-left: $grid-unit-15 / 2; // 6px.
padding-right: $grid-unit-15 / 2;

svg {
min-width: $button-size-small; // This is the optimal icon size, and we size the whole button after this.
}

&::before {
left: 2px;
right: 2px;
}
}

// First button in a group.
.components-toolbar div:first-child .components-button {
min-width: $block-toolbar-height - $grid-unit-15 / 2;
padding-left: $grid-unit-15 - $border-width;
padding-right: $grid-unit-15 / 2;

&::before {
left: $grid-unit-10;
right: 2px;
}
}

// Last button in a group.
.components-toolbar div:last-child .components-button {
min-width: $block-toolbar-height - $grid-unit-15 / 2;
padding-left: $grid-unit-15 / 2;
padding-right: $grid-unit-15 - $border-width;

&::before {
left: 2px;
right: $grid-unit-10;
}
}

// Single buttons should remain 48px.
.components-toolbar div:first-child:last-child > .components-button {
min-width: $block-toolbar-height;
padding-left: $grid-unit-15;
padding-right: $grid-unit-15;

&::before {
left: $grid-unit-10;
right: $grid-unit-10;
}
// Override Toolbar buttons size.
.components-toolbar-group,
.components-toolbar {
display: flex;
flex-wrap: nowrap;
}
}

Expand Down
5 changes: 3 additions & 2 deletions packages/components/src/toolbar-button/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ function ToolbarButton( {
isPressed={ props.isActive }
disabled={ props.isDisabled }
{ ...extraProps }
/>
{ children }
>
{ children }
Copy link
Member

@obenland obenland Apr 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@youknowriad Is it possible that this now overrides what's being passed in extraProps.children?
If you passed a button text to the toolbar previous to 7.9 that button now appears empty: Automattic/jetpack#15459

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For me the change here is not to avoid passing "children" but moving the children position inside the button which makes more sense IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That said, I'm still wondering why it regressed, the children should still show up for toolbar buttons? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

It looks like the "inner" children supersedes the children that is passed as a prop?

We fixed it in Automattic/jetpack#15508 through emulating how MediaReplaceFlow does it, so don't feel inclined to change anything on behalf of this. Awareness might be enough in case others run into this at some point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can also switch the priorities of the passed children (give the extraProps more priority), it is also weird to have the same prop in two different places, we should consolidate maybe. I wonder where the "extraProp" prop is passed and in which case the children should be extracted and just passed as regular "children" prop.

</Button>
</ToolbarButtonContainer>
);
}
Expand Down
71 changes: 66 additions & 5 deletions packages/components/src/toolbar-group/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
.components-toolbar-group {
border: $border-width solid $light-gray-500;
min-height: $block-toolbar-height;
border-right: $border-width solid $dark-gray-primary;
background-color: $white;
display: flex;
display: inline-flex;
flex-shrink: 0;
margin-right: -$border-width;
flex-wrap: wrap;

& & {
border-width: 0;
Expand All @@ -16,11 +17,14 @@
// Legacy toolbar group
// External code references to it, so we can't change it?
.components-toolbar {
min-height: $block-toolbar-height;
margin: 0;
border: $border-width solid $light-gray-500;
border: $border-width solid $dark-gray-primary;
border-radius: $radius-block-ui;
background-color: $white;
display: flex;
display: inline-flex;
flex-shrink: 0;
flex-wrap: wrap;
}

div.components-toolbar {
Expand Down Expand Up @@ -54,3 +58,60 @@ div.components-toolbar {
}
}
}

// Size multiple sequential buttons to be optically balanced.
// Icons are 36px, as set by a 24px icon and 12px padding.
.components-accessible-toolbar .components-toolbar-group > .components-button.components-button.has-icon,
.components-toolbar div > .components-button.components-button.has-icon {
min-width: $block-toolbar-height - $grid-unit-15;
padding-left: $grid-unit-15 / 2; // 6px.
padding-right: $grid-unit-15 / 2;

svg {
min-width: $button-size-small; // This is the optimal icon size, and we size the whole button after this.
}

&::before {
left: 2px;
right: 2px;
}
}

// First button in a group.
.components-accessible-toolbar .components-toolbar-group > .components-button:first-child.has-icon,
.components-toolbar div:first-child .components-button.has-icon {
min-width: $block-toolbar-height - $grid-unit-15 / 2;
padding-left: $grid-unit-15 - $border-width;
padding-right: $grid-unit-15 / 2;

&::before {
left: $grid-unit-10;
right: 2px;
}
}

// Last button in a group.
.components-accessible-toolbar .components-toolbar-group > .components-button:last-child.has-icon,
.components-toolbar div:last-child .components-button.has-icon {
min-width: $block-toolbar-height - $grid-unit-15 / 2;
padding-left: $grid-unit-15 / 2;
padding-right: $grid-unit-15 - $border-width;

&::before {
left: 2px;
right: $grid-unit-10;
}
}

// Single buttons should remain 48px.
.components-accessible-toolbar .components-toolbar-group > .components-button:first-child:last-child.has-icon,
.components-toolbar div:first-child:last-child > .components-button.has-icon {
min-width: $block-toolbar-height;
padding-left: $grid-unit-15;
padding-right: $grid-unit-15;

&::before {
left: $grid-unit-10;
right: $grid-unit-10;
}
}
Loading