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

Set font styles from block appender placeholder to inherit #21725

Merged
merged 2 commits into from
Apr 22, 2020

Conversation

Aljullu
Copy link
Contributor

@Aljullu Aljullu commented Apr 20, 2020

Description

Fixes #21724.

This PR sets font-family and font-size properties to inherit, so they don't force a specific font style that themes need to overwrite.

How has this been tested?

Tested how the block appender looks in many themes (Twenty Twenty, Twenty Nineteen, Twenty Seventeen, Twenty Fifteen and Storefront) to verify #21724 is fixed in the themes affected by it and that there are no regressions in the other themes. Tested in Firefox and IE11 to ensure #8654 wasn't reintroduced.

Screenshots

Using Twenty Seventeen (even though some other themes are affected as well):
Before:
Peek 2020-04-20 12-46

After:
Peek 2020-04-20 12-57

Types of changes

CSS bug fix.

@Aljullu Aljullu self-assigned this Apr 20, 2020
@github-actions
Copy link

github-actions bot commented Apr 20, 2020

Size Change: +19 B (0%)

Total Size: 842 kB

Filename Size Change
build/annotations/index.js 3.62 kB +2 B (0%)
build/api-fetch/index.js 4.08 kB +72 B (1%)
build/block-directory/index.js 6.24 kB -1 B
build/block-editor/index.js 105 kB +13 B (0%)
build/block-editor/style-rtl.css 10.1 kB -20 B (0%)
build/block-editor/style.css 10.1 kB -21 B (0%)
build/block-library/index.js 112 kB +213 B (0%)
build/block-library/style-rtl.css 7.17 kB +5 B (0%)
build/block-library/style.css 7.18 kB +5 B (0%)
build/blocks/index.js 57.7 kB -1 B
build/components/index.js 198 kB +7 B (0%)
build/data-controls/index.js 1.25 kB +1 B
build/edit-post/index.js 27.9 kB +2 B (0%)
build/edit-post/style-rtl.css 12.3 kB +54 B (0%)
build/edit-post/style.css 12.3 kB +55 B (0%)
build/edit-site/style-rtl.css 5.04 kB +5 B (0%)
build/edit-site/style.css 5.04 kB +6 B (0%)
build/edit-widgets/index.js 7.5 kB +5 B (0%)
build/edit-widgets/style-rtl.css 4.67 kB +5 B (0%)
build/edit-widgets/style.css 4.66 kB +5 B (0%)
build/editor/index.js 43.3 kB -1 B
build/editor/style-rtl.css 3.28 kB -198 B (6%)
build/editor/style.css 3.27 kB -198 B (6%)
build/element/index.js 4.65 kB +3 B (0%)
build/format-library/index.js 7.32 kB +2 B (0%)
build/keyboard-shortcuts/index.js 2.51 kB -1 B
build/list-reusable-blocks/index.js 3.12 kB -1 B
build/media-utils/index.js 5.29 kB -1 B
build/nux/index.js 3.4 kB +1 B
build/primitives/index.js 1.49 kB +1 B
build/rich-text/index.js 14.8 kB -2 B (0%)
build/server-side-render/index.js 2.67 kB +2 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 761 B 0 B
build/block-library/editor-rtl.css 7.1 kB 0 B
build/block-library/editor.css 7.1 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/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/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-site/index.js 10.5 kB 0 B
build/editor/editor-styles-rtl.css 428 B 0 B
build/editor/editor-styles.css 431 B 0 B
build/escape-html/index.js 733 B 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/keycodes/index.js 1.94 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/notices/index.js 1.79 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/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 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

@Aljullu Aljullu changed the title Remove overspecific font styles to block appender placeholder Remove overspecific font styles from block appender placeholder Apr 20, 2020
@youknowriad youknowriad requested a review from jasmussen April 21, 2020 14:34
@jasmussen
Copy link
Contributor

Hello @Aljullu! Thank you for this PR! Incidentally I just created a PR that is similar to this one and which fixes the same issue: #21749. Only instead of removing the font-family and font-size definitions, it explicitlhy sets them to inherit from the editor-styles-wrapper element, like so:

	textarea.block-editor-default-block-appender__content { // Needs specificity in order to override input field styles from WP-admin styles.
		font-family: inherit;
		font-size: inherit;
		border: none;
		background: none;
...

In my tests that actually fixes the issue correctly: keeps Noto Serif when no editor style is registered, but properly inherits the font assigned to body when a theme does register an editor style. When I test this branch, with the rules just remove, I see this:

Screenshot 2020-04-22 at 09 12 45

That is, the font reverts to the system font and default system font size.

However, your PR was first :) You deserve the props! If you like, you can include the inherit rules, update this PR and merge to fix the issue you created.

I will then rebase my PR off of yours, because my PR also adds inheritance to the Title block. What do you think?

@Aljullu Aljullu changed the title Remove overspecific font styles from block appender placeholder Set font styles from block appender placeholder to inherit Apr 22, 2020
@Aljullu
Copy link
Contributor Author

Aljullu commented Apr 22, 2020

However, your PR was first :) You deserve the props!

Ha, no problem with the props. 🙂 I updated this PR with your feedback, but feel free to merge yours directly if that's easier.

@jasmussen
Copy link
Contributor

The rebase is trivial! I'll give this PR a good testing in various themes and approve in a little bit.

Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

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

This is solid. Here's what I see on a fresh page in...

A theme I'm working on:

Screenshot 2020-04-22 at 15 38 06

No editor style registered (vanilla):

Screenshot 2020-04-22 at 15 38 16

TwentyEleven:

twentyeleven

TwentyNineteen:

Screenshot 2020-04-22 at 15 39 10

TwentyTwenty is doing it wrong, but at least it's no different from before:

Screenshot 2020-04-22 at 15 38 44

Screenshot 2020-04-22 at 15 38 56

@jasmussen jasmussen merged commit d7a1f17 into master Apr 22, 2020
@jasmussen jasmussen deleted the fix/21724-remove-css-font-overrides branch April 22, 2020 13:48
@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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New paragraph placeholder changes font-style/size after focus in many themes
2 participants