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.

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

Allow title and button-based appender to inherit styles. #21749

Merged
merged 1 commit into from
Apr 22, 2020

Conversation

jasmussen
Copy link
Contributor

File under "should've probably done this sooner", this PR allows the title field and the initial appender to inherit the theme styles defined by an editor style.

In the past this wasn't possible due to how things were structured. But as it is, those styles are inherited by what is applied to the editor-styles-wrapper, which means this works both for themes that don't register and editor style — it looks like before — but now it also works for themes that do.

Before, vanilla style:

before

After, vanilla style:

vanilla

After, with the following CSS registered as an editor style:

body {
	font-family: "Muli", sans-serif;
	font-size: 18px;
}

customized

Because the title is not inside an h1 element, we can't yet inherit font sizes and weights there. But this will be fixed once the title element becomes an actual block.

@jasmussen jasmussen self-assigned this Apr 21, 2020
@jasmussen jasmussen added [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. [Feature] Custom Editor Styles Functionality for adding custom editor styles labels Apr 21, 2020
@github-actions
Copy link

github-actions bot commented Apr 21, 2020

Size Change: -22 B (0%)

Total Size: 842 kB

Filename Size Change
build/block-editor/style-rtl.css 10.1 kB -1 B
build/block-editor/style.css 10.1 kB -1 B
build/editor/style-rtl.css 3.27 kB -10 B (0%)
build/editor/style.css 3.27 kB -10 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 4.08 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.24 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 761 B 0 B
build/block-editor/index.js 105 kB 0 B
build/block-library/editor-rtl.css 7.13 kB 0 B
build/block-library/editor.css 7.13 kB 0 B
build/block-library/index.js 112 kB 0 B
build/block-library/style-rtl.css 7.19 kB 0 B
build/block-library/style.css 7.19 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.7 kB 0 B
build/components/index.js 198 kB 0 B
build/components/style-rtl.css 16.9 kB 0 B
build/components/style.css 16.9 kB 0 B
build/compose/index.js 6.66 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.25 kB 0 B
build/data/index.js 8.43 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/index.js 3.54 kB 0 B
build/edit-navigation/style-rtl.css 485 B 0 B
build/edit-navigation/style.css 485 B 0 B
build/edit-post/index.js 27.9 kB 0 B
build/edit-post/style-rtl.css 12.3 kB 0 B
build/edit-post/style.css 12.3 kB 0 B
build/edit-site/index.js 10.5 kB 0 B
build/edit-site/style-rtl.css 5.04 kB 0 B
build/edit-site/style.css 5.04 kB 0 B
build/edit-widgets/index.js 7.5 kB 0 B
build/edit-widgets/style-rtl.css 4.67 kB 0 B
build/edit-widgets/style.css 4.66 kB 0 B
build/editor/editor-styles-rtl.css 428 B 0 B
build/editor/editor-styles.css 431 B 0 B
build/editor/index.js 43.3 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.32 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.67 kB 0 B
build/primitives/index.js 1.49 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.67 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@draganescu
Copy link
Contributor

So, I tried to test:

  • local environment
  • switched to this branch
  • using TwentyTwenty
  • adding the snippet to assets/css/editor-style-block.css

Nothing happens. It is b/c of the theme?

@jasmussen
Copy link
Contributor Author

Thank you for looking! And sorry for not including better steps to test.

TwentyTwenty is not loading editor styles correctly, so it will not make a difference there.

  1. The first thing to test is a theme that doesn't load an editor style, could be any old WordPress theme. Or https://github.com/jasmussen/navi if you need something specific. In this theme, the title and empty appender on a completely fresh post should look the same before and after this branch, that is set in the Noto Serif font.

  2. The next thing to test is a theme that correctly loads an editor style, such as TwentyNineteen. Verify that on a fresh empty post, the title and appender look correctly styled to that theme.

  3. The final thing to test is perhaps optional, but would be good to verify — edit the TwentyNineteen editor style to see if you can style the title and appender using only the following CSS:

body {
	font-family: "Comic Sans MS", cursive, sans-serif;
	font-size: 48x;
}

If all looks correct, this should be good to go :)

@jasmussen
Copy link
Contributor Author

Fixes #21724.

@jasmussen
Copy link
Contributor Author

I'm getting a bunch of failed e2e tests on this one, which seem completely unrelated to the fact that this is just CSS. Any insights?

@karmatosed karmatosed self-requested a review April 22, 2020 13:20
Copy link
Member

@karmatosed karmatosed left a comment

Choose a reason for hiding this comment

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

Approving based on design review. Simple change so works for me.

@karmatosed karmatosed removed the Needs Design Feedback Needs general design feedback. label Apr 22, 2020
File under "should've probably done this sooner", this PR allows the title field and the initial appender to inherit the theme styles defined by an editor style.

In the past this wasn't possible due to how things were structured. But as it is, those styles are inherited by what is applied to the `editor-styles-wrapper`, which means this works both for themes that don't register and editor style — it looks like before — but now it also works for themes that do.
@jasmussen
Copy link
Contributor Author

Thanks for the approval. I've tested this well and it's a trivial code change so I'm going to go ahead and merge.

@jasmussen jasmussen merged commit 0b6ae94 into master Apr 22, 2020
@jasmussen jasmussen deleted the try/appender-inherit-fonts branch April 22, 2020 15:11
@github-actions github-actions bot added this to the Gutenberg 8.0 milestone Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Custom Editor Styles Functionality for adding custom editor styles [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants