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

Preformatted block: Add support for font sizes #27584

Merged
merged 8 commits into from
Dec 15, 2020
Merged

Preformatted block: Add support for font sizes #27584

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
86b553b
Add preformatted block styles and font settings.
mendezcode Dec 8, 2020
33e6b85
Add font size fallback.
mendezcode Dec 8, 2020
4dafabe
Remove unnecessary overflow property and use `pre-wrap` instead.
mendezcode Dec 9, 2020
93d7527
Remove editor.scss
mendezcode Dec 10, 2020
c072553
Remove CSS custom property.
mendezcode Dec 14, 2020
486ec4a
Add stylesheet to `block.json` file
mendezcode Dec 15, 2020
5b789c3
Add preformatted block to theme-json.md specs.
mendezcode Dec 15, 2020
a331c3e
Fix preformatted block support information on theme-json.md specs.
mendezcode Dec 15, 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
1 change: 1 addition & 0 deletions packages/block-library/src/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
@import "./post-content/editor.scss";
@import "./post-excerpt/editor.scss";
@import "./post-author/editor.scss";
@import "./preformatted/editor.scss";
@import "./pullquote/editor.scss";
@import "./quote/editor.scss";
@import "./rss/editor.scss";
Expand Down
3 changes: 2 additions & 1 deletion packages/block-library/src/preformatted/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
}
},
"supports": {
"anchor": true
"anchor": true,
"fontSize": true
}
mendezcode marked this conversation as resolved.
Show resolved Hide resolved
}
4 changes: 4 additions & 0 deletions packages/block-library/src/preformatted/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.wp-block-preformatted {
overflow-x: auto;
Copy link
Contributor

Choose a reason for hiding this comment

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

The style.scss file always gets loaded into the editor, so if a rule exists in that file, it does not need to be replicated in editor.scss.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, yes, true that. I'll fix accordingly.

white-space: pre !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this rule also exists in style.scss, we are so close to not needing any editor style at all for this block. And we should be avoiding !important anyway, if at all possible. I dug in to see why the !important was necessary, and it appears that white-space: pre-wrap; is applied as an inline style on the preformatted block:

Screenshot 2020-12-09 at 08 20 30

Why does that exist? I'm not sure, but from digging at the code, it looks to be related to this PR: #18656. Or possibly #17779. Specifically there's some commentary in https://github.com/WordPress/gutenberg/pull/17779/files#diff-7df6f5253ccec2f2e6d1bb22e81f87a4b6bccd2604463eeecded163aa27fd333R70 which seems relevant.

It seems we need to figure out why that output

Copy link
Contributor Author

@mendezcode mendezcode Dec 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The pre-wrap inline style is coming from the <RichText> component, and its value is defined here: https://github.com/WordPress/gutenberg/blob/master/packages/rich-text/src/component/index.js#L82

The value is set on the whiteSpace variable, which is then added to defaultStyle, which subsequently makes it to the component's inline styles via the style prop here: https://github.com/WordPress/gutenberg/blob/master/packages/rich-text/src/component/index.js#L1075

According to the inspector, I can see that besides the pre-wrap inline style, there's the pre-wrap style attribute coming from the Block's style.scss stylesheet. If neither the inline style nor the block style are present, then, the <pre> element will get its default style from the browser defaults.

This makes me think we can live with the inline style, since the <RichText> component is using the style attribute to guarantee its proper display (my assumption). Assuming a future scenario where the <RichText> component removes the pre-wrap inline style, then we'll still be covered by the pre-wrap style available in the preformatted block's style.scss.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just remove the inline style?

Copy link
Contributor Author

@mendezcode mendezcode Dec 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

That's a quite considerable change. The <RichText> component uses it by default, and it (the component) is used in many places. Changing this may change the overall behavior of the <RichText> component.

}
5 changes: 5 additions & 0 deletions packages/block-library/src/preformatted/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
.wp-block-preformatted {
font-size: var(--wp--preset--font-size--extra-small, 1em);
Copy link
Member

Choose a reason for hiding this comment

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

👋 We should remove the CSS Custom Property from here. I've seen that the code block recently got it added as well in https://github.com/WordPress/gutenberg/pull/27294/files#r540890153 and I've prepared a PR that fixes and explain why we shouldn't do that #27672

overflow-x: auto;
white-space: pre;
}
1 change: 1 addition & 0 deletions packages/block-library/src/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
@import "./navigation-link/style.scss";
@import "./paragraph/style.scss";
@import "./post-author/style.scss";
@import "./preformatted/style.scss";
@import "./pullquote/style.scss";
@import "./query-loop/style.scss";
@import "./quote/style.scss";
Expand Down