Reduce paragraph block CSS specificity by m-e-h · Pull Request #13025 · WordPress/gutenberg

m-e-h · 2018-12-19T17:20:03Z

Description

Move drop-cap:focus style to editor.css.
Reduce specificity on front-end drop-cap by removing the :not(:focus) pseudo-class.
Show a visual hint that a dropcap is present while focused by:

make the first letter a little larger, 1.2em
make the first letter bolder by using a text-shadow of the same color
keep the uppercase text-transform.

How has this been tested?

Visual tests in browser.

Screenshots

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.

m-e-h · 2018-12-19T22:06:29Z

This PR also helps to chip away at some of
#11677
and some of
#11668

jorgefilipecosta

Hi @m-e-h, I agree with the logic of these changes, I think focus styles that are editor specific should not be referenced on the front end styles.
Unfortunately, I think we will not be able to differentiate the first letter with this styles, as even this simple styles change causes the bug shown in the following gif:

m-e-h · 2018-12-20T15:54:40Z

Thanks for looking at this @jorgefilipecosta!

That appears to be a Chrome bug. Works fine in Firefox.
I'll see if I can find out what's going on.

m-e-h · 2018-12-20T16:18:11Z

Yep, it's a known bug in Chrome. https://bugs.chromium.org/p/chromium/issues/detail?id=174349
Probably accounts for some of the earlier issues the editor had with Drop cap.

The cursor bug becomes present even with an empty rule in the stylesheet:

.has-drop-cap:focus::first-letter {}

So it looks like :not(:focus) is the only solution. At least until this gets fixed in Chrome.

m-e-h · 2018-12-21T00:49:00Z

Though it would be nice to ultimately remove that :not(:focus) style from the front end, I don't see any way around it in the foreseeable future. Not much activity on that Chromium bug page.

Should I close this?

One sort of separate thing I'd also like to do here with the dropcap styles is remove the p qualifier preceding each selector.

Does anyone know if there's a reason for it being there?

jasmussen · 2019-02-08T09:33:33Z

Nice work here. Some GIFs will help.

This branch:

Master:

I appreciate what you're trying to do here, notably the reduction in CSS specificity. But the bold first letter goes a little bit against the principle that the unselected block is a preview and the selected block can be an edit mode. I deeply appreciate what you're trying to do here, but honestly I feel like what we have in master with regards to the select and edit behavior is the better solution.

But the change to CSS specificity in style.scss is a total no-brainer.

Can you change this PR so the only change is the reduction in CSS specificity in the style.scss file? I think that'd be worth shipping.

…l-on-focus

m-e-h · 2019-02-08T20:13:39Z

I removed the dropcap changes that I tried on the editor.scss.
I removed the qualifying p tag from each selector in style.scss, thereby decreasing the specificity.

But @jasmussen I'm bringing back the editor dropcap argument if and when Chrome ever fixes it's :focus:first-letter bug. 😃

kjellr

Looks good, thanks @m-e-h!

ZebulanStanphill · 2019-02-11T17:46:49Z

packages/block-library/src/paragraph/style.scss

-	&.is-small-text {
-		font-size: 14px;
-	}
+.is-small-text {


Isn't this selector a bit too generic? What about other blocks that use a class with this same name? Shouldn't this class be either namespaced (e.g. .wp-block-paragraph__is-small-text or changed to something along the lines of .wp-block-paragraph.is-small-text?

Probably should've been .wp-block-paragraph__is-small-text.
In the case of this PR though, does the p qualifier really make it any less generic?

I think .wp-block-paragraph.is-small-text is too specific for a core style. Especially with something like font-size which is likely one of the first things that a theme will override.

For the record though, I would prefer .wp-block-paragraph.is-small-text over p.is-small-text. Since there are more appropriate hooks in place for themes to override the Paragraph Block font-sizes.

As for being generic, the one that sticks out to me here is .has-background. That's not even "text" specific and we're adding some hefty padding here. Is that class used in any other blocks?

Dang! This is going to have to be fixed. The Button block also uses .has-background and is getting the padding from the Paragraph block CSS. Guess We're going to have to increase specificity for the paragraph block rather than decrease.

@kjellr How should I go about fixing? A new PR?

Ah yes, good catch. I missed that selector. Open up a new PR and I'll take a look shortly.

I think the has-background and has-text-color selectors are the only ones that definitely should be changed for now. I don't believe the text size and dropcap ones are used elsewhere.

Had a few minutes, so I opened #13821 to take care of this. 👍

Thanks @kjellr!
Totally my fault. The PR was originally for the Dropcap and sort of morphed into being about specificity. I didn't put as much thought into it as I should have.

I agree that the other selectors don't look as problematic as has-background. Even if something like is-small-text is used in other blocks, we're only setting the font size, which would be expected for such a class name and would likely be the same or overridden by the other block.

It's the padding on a generic "background" selector that made this troublesome.
It could be debated that the padding style is more appropriate in theme.css. But that's a topic for another day. 😄

You're all amazing. Thanks for the hard work here.

Totally my fault. The PR was originally for the Dropcap and sort of morphed into being about specificity. I didn't put as much thought into it as I should have.

No problem at all! It's all set now. Thanks again for your contribution, and thanks @ZebulanStanphill for starting the discussion that led to us catching this. 👍

This extra specificity was removed in #13025, but should be restored because other blocks (buttons, pullquotes, etc), use the `has-background` and `has-text-color` classes. We don't want the styles here to interfere. Using a `p` selector here, rather than something like `.wp-block-paragraph` so that this gets picked up correctly on the front-end as well.

youknowriad · 2019-02-12T09:51:55Z

packages/block-library/src/paragraph/style.scss

-	}
+.is-larger-text {
+	font-size: 48px;
+}


Is this the right file for these stylesheets? If I remember properly, these are theme options + editor styles? shouldn't we move these styles to the build-int editor-styles.css instead?

…13821) This extra specificity was removed in #13025, but should be restored because other blocks (buttons, pullquotes, etc), use the `has-background` and `has-text-color` classes. We don't want the styles here to interfere. Using a `p` selector here, rather than something like `.wp-block-paragraph` so that this gets picked up correctly on the front-end as well.

* move dropcap focus to editor.css. Show hint of DC on focus * Move dropcap focus to editor.css. Reduce specificity * Easing up on paragraph block specificity.

…13821) This extra specificity was removed in #13025, but should be restored because other blocks (buttons, pullquotes, etc), use the `has-background` and `has-text-color` classes. We don't want the styles here to interfere. Using a `p` selector here, rather than something like `.wp-block-paragraph` so that this gets picked up correctly on the front-end as well.

* move dropcap focus to editor.css. Show hint of DC on focus * Move dropcap focus to editor.css. Reduce specificity * Easing up on paragraph block specificity.

…13821) This extra specificity was removed in #13025, but should be restored because other blocks (buttons, pullquotes, etc), use the `has-background` and `has-text-color` classes. We don't want the styles here to interfere. Using a `p` selector here, rather than something like `.wp-block-paragraph` so that this gets picked up correctly on the front-end as well.

m-e-h added 2 commits December 19, 2018 12:12

move dropcap focus to editor.css. Show hint of DC on focus

fbcb42b

Move dropcap focus to editor.css. Reduce specificity

b23a3e4

m-e-h changed the title ~~Fix/dropcap visual on focus~~ Drop cap visual on focused editor Dec 19, 2018

jorgefilipecosta reviewed Dec 20, 2018

View reviewed changes

gziolo added [Block] Paragraph Affects the Paragraph Block Needs Design Feedback Needs general design feedback. labels Feb 7, 2019

gziolo added this to the 5.1 (Gutenberg) milestone Feb 7, 2019

gziolo requested review from jasmussen and kjellr February 7, 2019 13:09

Merge remote-tracking branch 'upstream/master' into fix/dropcap-visua…

b851085

…l-on-focus

m-e-h requested review from Soean, ajitbohra, gziolo, noisysocks, notnownikki, talldan and youknowriad as code owners February 8, 2019 17:19

Easing up on paragraph block specificity.

8fda064

m-e-h changed the title ~~Drop cap visual on focused editor~~ Reduce paragraph block CSS specificity Feb 8, 2019

kjellr approved these changes Feb 11, 2019

View reviewed changes

kjellr merged commit a4412e6 into WordPress:master Feb 11, 2019

ZebulanStanphill reviewed Feb 11, 2019

View reviewed changes

kjellr mentioned this pull request Feb 11, 2019

Restore more specific CSS for paragraph block custom color classes #13821

Merged

youknowriad reviewed Feb 12, 2019

View reviewed changes

This was referenced Feb 18, 2019

Bump plugin version to 5.1.0-rc.1 #13908

Merged

Bump plugin version to 5.1.0 #13960

Merged

youknowriad pushed a commit that referenced this pull request Mar 6, 2019

Reduce paragraph block CSS specificity (#13025)

cef4e61

* move dropcap focus to editor.css. Show hint of DC on focus * Move dropcap focus to editor.css. Reduce specificity * Easing up on paragraph block specificity.

youknowriad pushed a commit that referenced this pull request Mar 6, 2019

Reduce paragraph block CSS specificity (#13025)

27b51ba

* move dropcap focus to editor.css. Show hint of DC on focus * Move dropcap focus to editor.css. Reduce specificity * Easing up on paragraph block specificity.

phcp mentioned this pull request Oct 17, 2024

Drop Cap Causes Inconsistent Behavior in Editor After Switching Between Paragraphs Automattic/wp-calypso#95397

Open

Conversation

m-e-h commented Dec 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

Uh oh!

m-e-h commented Dec 19, 2018

Uh oh!

jorgefilipecosta left a comment

Choose a reason for hiding this comment

Uh oh!

m-e-h commented Dec 20, 2018

Uh oh!

m-e-h commented Dec 20, 2018

Uh oh!

m-e-h commented Dec 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasmussen commented Feb 8, 2019

Uh oh!

m-e-h commented Feb 8, 2019

Uh oh!

kjellr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m-e-h Feb 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

m-e-h commented Dec 19, 2018 •

edited

Loading

m-e-h commented Dec 21, 2018 •

edited

Loading

m-e-h Feb 11, 2019 •

edited

Loading