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

Reduce the specificity of block styles to simplify editor styles slightly #14407

Merged
merged 3 commits into from
Apr 17, 2019
Merged

Reduce the specificity of block styles to simplify editor styles slightly #14407

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
2cd2b3a
Try: Remove intrinsic block margins, rely on cascade
jasmussen Mar 13, 2019
2361a77
Try a new "Safety Margin"
jasmussen Apr 2, 2019
8d3cea6
Add top margin also.
Apr 9, 2019
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
1 change: 1 addition & 0 deletions assets/stylesheets/_variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ $default-line-height: 1.4;
$editor-font: "Noto Serif", serif;
$editor-html-font: Menlo, Consolas, monaco, monospace;
$editor-font-size: 16px;
$default-block-margin: 28px; // This value provides a consistent, contiguous spacing between blocks (it's 2x $block-padding).
$text-editor-font-size: 14px;
$editor-line-height: 1.8;
$big-font-size: 18px;
Expand Down
9 changes: 0 additions & 9 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 @@ -56,15 +56,6 @@
margin-left: -$block-padding;
margin-right: -$block-padding;
}

// Space every block, and the default appender, using margins.
// This allows margins to collapse, which gives a better representation of how it looks on the frontend.
.block-editor-default-block-appender > .block-editor-default-block-appender__content,
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if having the margins here is a bad thing. I do understand the reasoning behind moving them to the individual block to allow them to be overridden there. But were there really a lot of complaints about the margins not matching up with the front-end?
Like I've said before, I think a little variance is expected in the administrative UI. Enough space for controls to work is absolutely a good reason for this.

That said, I think the space could be reduced from 32px.
The element's margins will take over as long as they're greater than the .block-editor-block-list__block-edit margins.

For example if a paragraph's p tag has a margin-bottom: 40px, then that block will have 40px margin between it and the next block in the editor. If it has margin-bottom: 16px, then it'll have the "fallback" margin from .block-editor-block-list__block-edit of 32px.
So the trick is to define the minimum required margin for a good editing experience and let theme styles go bigger but not smaller.

In my short experimenting 24px seems to work well. (wish it was less, but it just isn't)
24px roughly equates to 1.5rem which is a common margin, I think, so that's good. I would avoid em because of it's unpredictability.

And @ellatrix has a good point. The top margin is a bit pointless. Only place it would be seen is on the :first-child, but a margin-bottom on .editor-post-title would accomplish the same.

Copy link
Contributor Author

@jasmussen jasmussen Mar 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

But were there really a lot of complaints about the margins not matching up with the front-end?

Well, see feedback on this thread ;)

Edit: to clarify — it's not about complaints that the margins didn't match up to the front-end, it was about the selector being too specific and therefore hard for themes to override. Did not mean to be sarcastic or snide in that comment above, sorry if it came off that way.

Like I've said before, I think a little variance is expected in the administrative UI. Enough space for controls to work is absolutely a good reason for this.

The ultimate goal is for the editor to be able to look exactly like the frontend with minimal work by the theme editor. Every research we've conducted suggests that when there's a strong visual connection between the two, usability increases. I agree, a little variance is acceptable, but I also feel the more we can do to reduce this variance, the better. The difficult balance is to ensure a good editing experience for themes that do not style the editor.

To the rest of your feedback, very solid, thank you, and I agree with Ella too. Will take a stab at addressing both of your points.

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 tried to chagne the 32px to 24px. But it starts to affect the multi-select appearance.

32px:

Screenshot 2019-03-29 at 11 11 52

24px:

Screenshot 2019-03-29 at 11 15 56

What happens there is that every block has a footprint, and outside that footprint, the block borders are drawn. By default this block border is drawn 14px out, which means two blocks next to each other, both multi-selected, will have 14px + 14px of "selection rectangle" drawn around them. Which means if the margin between the two blocks was 28px, they would appear flush next to each other. At 32px there's 4px gap. At 24px, there's a 4px overlap which is what you're seeing in the second screenshot.

All that is to say: we can change that. There's a $block-padding variable that's trivial to change. If 24px is the magic number, we can set the block padding to 10 or 12px.

But this is a big visual change, and I would love to ship this PR before exploring such a change. Do you think we could explore such a margin and padding change separately?

Copy link
Member

Choose a reason for hiding this comment

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

You've been busy today @jasmussen ! 😄

it was about the selector being too specific and therefore hard for themes to override.

You're right. In my idealistic mind, themes shouldn't need to think about overriding anything and selectors like .block-editor-block-list__block-edit shouldn't ever be a part of a theme's vocabulary.
We're lucky enough to have a lot of semantic HTML5 element wrappers on our blocks. These elements are often the first things to get styled in themes, old and new.

p,
ul,
blockquote,
table,
figure {
    margin: 0 0 1rem;
}

That's free backward compatibility if we can take advantage of it.

Do you think we could explore such a margin and padding change separately?

Definitely. I think setting the minimum spacing on this wrapping editor element will be key to making this work well. It will act as a safety net in case somebody is using a crazy Meyer type reset but it'll allow a themes vertical rhythm to shine through in most cases.

Copy link
Contributor Author

@jasmussen jasmussen Mar 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Definitely. I think setting the minimum spacing on this wrapping editor element will be key to making this work well. It will act as a safety net in case somebody is using a crazy Meyer type reset but it'll allow a themes vertical rhythm to shine through in most cases.

I think this is probably a key insight. @kjellr suggested something slightly similar in a recent chat. In #14407 (comment) I tried a generic rule that didn't quite work out. But maybe this idea can inspire a better rule?

> .block-editor-block-list__block > .block-editor-block-list__block-edit,
> .block-editor-block-list__layout > .block-editor-block-list__block > .block-editor-block-list__block-edit {
margin-top: $block-padding * 2 + $block-spacing;
margin-bottom: $block-padding * 2 + $block-spacing;
}
}

.block-editor-block-list__layout .block-editor-block-list__block {
Expand Down
2 changes: 2 additions & 0 deletions packages/block-editor/src/components/default-block-appender/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
outline: $border-width solid transparent;
transition: 0.2s outline;
resize: none;
margin-top: $default-block-margin;
margin-bottom: $default-block-margin;

// Emulate the dimensions of a paragraph block.
// On mobile and in nested contexts, the plus to add blocks shows up on the right.
Expand Down
7 changes: 5 additions & 2 deletions packages/block-library/src/button/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,17 @@
}

.wp-block-button {
display: inline-block;
margin-bottom: 0;
position: relative;

[contenteditable] {
cursor: text;
}

// Don't let placeholder expand parent width.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When this was applied to the button itself, it prevented the margin collapse. By moving it down here, the net result is the same: an empty button isn't 100% wide, but it looks the same in frontend and backend.

.block-editor-rich-text {
display: inline-block;
}

// Make placeholder text white unless custom colors or outline versions are chosen.
&:not(.has-text-color):not(.is-style-outline) .block-editor-rich-text__editable[data-is-placeholder-visible="true"] + .block-editor-rich-text__editable {
color: $white;
Expand Down
1 change: 0 additions & 1 deletion packages/block-library/src/button/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ $blocks-button__height: 56px;

.wp-block-button {
color: $white;
margin-bottom: 1.5em;

&.aligncenter {
text-align: center;
Expand Down
16 changes: 11 additions & 5 deletions packages/block-library/src/columns/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,18 @@
margin-left: -$border-width;
}

// The Column block is a child of the Columns block and is mostly a passthrough container.
// Therefore it shouldn't add additional paddings and margins, so we reset these, and compensate for margins.
// @todo In the future, if a passthrough feature lands, it would be good to apply these rules to such an element in a more generic way.
// Zero out margins.
> [data-block] {
margin-top: 0;
margin-bottom: 0;
}

// The Columns block is a flex-container, therefore it nullifies margin collapsing.
// Therefore, blocks inside this will appear to create a double margin.
// We compensate for this using negative margins.
> div > .block-core-columns > .editor-inner-blocks {
margin-top: -$block-padding - $block-padding;
margin-bottom: -$block-padding - $block-padding;
margin-top: -$default-block-margin;
margin-bottom: -$default-block-margin;
}
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/columns/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
.wp-block-columns {
display: flex;
margin-bottom: $default-block-margin;

// Responsiveness: Allow wrapping on mobile.
flex-wrap: wrap;
Expand Down
1 change: 0 additions & 1 deletion packages/block-library/src/cover/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
background-position: center center;
min-height: 430px;
width: 100%;
margin: 0 0 1.5em 0;
display: flex;
justify-content: center;
align-items: center;
Expand Down
16 changes: 15 additions & 1 deletion packages/block-library/src/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
@import "./classic/editor.scss";
@import "./gallery/editor.scss";
@import "./group/editor.scss";
@import "./heading/editor.scss";
@import "./html/editor.scss";
@import "./image/editor.scss";
@import "./latest-comments/editor.scss";
Expand All @@ -34,3 +33,18 @@
@import "./text-columns/editor.scss";
@import "./verse/editor.scss";
@import "./video/editor.scss";


/**
* Editor Normalization Styles
*
* These are only output in the editor, but styles here are NOT prefixed .editor-styles-wrapper.
* This allows us to create normalization styles that are easily overridden by editor styles.
*/

// Provide every block with a default base margin. This margin provides a consistent spacing
// between blocks in the editor.
[data-block] {
margin-top: $default-block-margin;
margin-bottom: $default-block-margin;
}
17 changes: 12 additions & 5 deletions packages/block-library/src/gallery/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
// Override the default list style type _only in the editor_
// to avoid :not() selector specificity issues.
// See https://github.com/WordPress/gutenberg/pull/10358
ul.wp-block-gallery li {
list-style-type: none;
ul.wp-block-gallery {
// Override the default list style type _only in the editor_
// to avoid :not() selector specificity issues.
// See https://github.com/WordPress/gutenberg/pull/10358
li {
list-style-type: none;
}

// Override the bottom margin every block gets.
.is-selected & {
margin-bottom: 0;
}
}

.blocks-gallery-item {
Expand Down
47 changes: 0 additions & 47 deletions packages/block-library/src/heading/editor.scss
View file
Open in desktop

This file was deleted.

30 changes: 17 additions & 13 deletions packages/block-library/src/html/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
.wp-block-html .block-editor-plain-text {
font-family: $editor-html-font;
color: $dark-gray-800;
padding: 0.8em 1em;
border: 1px solid $light-gray-500;
border-radius: 4px;
.wp-block-html {
margin-bottom: $default-block-margin;

/* Fonts smaller than 16px causes mobile safari to zoom. */
font-size: $mobile-text-min-font-size;
@include break-small {
font-size: $default-font-size;
}
.block-editor-plain-text {
font-family: $editor-html-font;
color: $dark-gray-800;
padding: 0.8em 1em;
border: 1px solid $light-gray-500;
border-radius: 4px;

/* Fonts smaller than 16px causes mobile safari to zoom. */
font-size: $mobile-text-min-font-size;
@include break-small {
font-size: $default-font-size;
}

&:focus {
box-shadow: none;
&:focus {
box-shadow: none;
}
}
}
2 changes: 2 additions & 0 deletions packages/block-library/src/more/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
.block-editor-block-list__block[data-type="core/more"] {
max-width: 100%;
text-align: center;
margin-top: $default-block-margin;
margin-bottom: $default-block-margin;
}

.block-editor .wp-block-more { // needs specificity
Expand Down
2 changes: 2 additions & 0 deletions packages/block-library/src/nextpage/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
.block-editor-block-list__block[data-type="core/nextpage"] {
max-width: 100%;
margin-top: $default-block-margin;
margin-bottom: $default-block-margin;
}

.wp-block-nextpage {
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/pullquote/theme.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
.wp-block-pullquote {
border-top: 4px solid $dark-gray-500;
border-bottom: 4px solid $dark-gray-500;
margin-bottom: $default-block-margin;
color: $dark-gray-600;

cite,
Expand Down
8 changes: 2 additions & 6 deletions packages/block-library/src/quote/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
.wp-block-quote {
margin: 0;

&__citation {
font-size: $default-font-size;
}
.wp-block-quote__citation {
font-size: $default-font-size;
}
2 changes: 1 addition & 1 deletion packages/block-library/src/quote/theme.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
.wp-block-quote {
border-left: 4px solid $black;
margin: 20px 0;
margin: 0 0 $default-block-margin 0;
padding-left: 1em;

cite,
Expand Down
3 changes: 2 additions & 1 deletion packages/block-library/src/separator/theme.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
.wp-block-separator {
border: none;
border-bottom: 2px solid $dark-gray-100;
margin: 1.65em auto;
margin-left: auto;
margin-right: auto;

// Default, thin style
&:not(.is-style-wide):not(.is-style-dots) {
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/shortcode/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
background-color: $dark-opacity-light-200;
font-size: $default-font-size;
font-family: $default-font;
margin-bottom: $default-block-margin;

label {
display: flex;
Expand Down
4 changes: 4 additions & 0 deletions packages/block-library/src/spacer/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
.block-library-spacer__resize-container.is-selected {
background: $light-gray-200;
}

.block-library-spacer__resize-container {
margin-bottom: $default-block-margin;
}
15 changes: 13 additions & 2 deletions packages/block-library/src/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,9 @@

/**
* Vanilla Block Styles
* These are base styles that apply across blocks.
* We should have as few of these as possible.
* These are base styles that apply across blocks, meant to provide a baseline.
* They are applied both to the editor and the theme, so we should have as few of these as possible.
* Please note that some styles are stored in packages/editor/src/editor-styles.scss, as they pertain to CSS bleed for the editor only.
*/

// Caption styles.
Expand All @@ -151,3 +152,13 @@
figcaption {
margin-top: 0.5em;
}

// Apply some base styles to blocks that need them.
img {
max-width: 100%;
height: auto;
}

iframe {
width: 100%;
}
13 changes: 9 additions & 4 deletions packages/components/src/placeholder/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
.components-placeholder {
margin: 0;
margin-bottom: $default-block-margin;
display: flex;
flex-direction: column;
align-items: center;
Expand All @@ -8,17 +8,22 @@
min-height: 200px;
width: 100%;
text-align: center;
font-family: $default-font;
font-size: $default-font-size;

// use opacity to work in various editor styles
// Use opacity to work in various editor styles.
background: $dark-opacity-light-200;

.is-dark-theme & {
background: $light-opacity-light-200;
}
}

.components-placeholder__instructions,
.components-placeholder__label,
.components-placeholder__fieldset {
font-family: $default-font;
font-size: $default-font-size;
}

.components-placeholder__label {
display: flex;
align-items: center;
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-post/src/components/text-editor/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
.editor-post-title__block {
textarea {
border: $border-width solid $light-gray-500;
margin-bottom: 4px;
margin-bottom: $block-spacing;
padding: $block-padding;

&:focus {
Expand Down
7 changes: 4 additions & 3 deletions packages/edit-post/src/components/visual-editor/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@
margin-left: auto;
margin-right: auto;

// Space title similarly to other blocks.
// This uses negative margins so as to not affect the default block margins.
margin-bottom: -$block-padding - $block-spacing - $border-width - $border-width;
// Apply default block margin below the post title.
// This ensures the first block on the page is in a good position.
// This rule can be retired once the title becomes an actual block.
margin-bottom: ($block-padding * 2) + $block-spacing; // This matches 2em in the vanilla style.

// Stack borders.
> div {
Expand Down
9 changes: 0 additions & 9 deletions packages/edit-post/src/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,6 @@ body.block-editor-page {
}
}

img {
max-width: 100%;
height: auto;
}

iframe {
width: 100%;
}

.components-navigate-regions {
height: 100%;
}
Expand Down
Loading