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

Enhancing Quote block styling for different text alignments #13248

Merged
merged 10 commits into from
Feb 12, 2019
Merged

Enhancing Quote block styling for different text alignments #13248

merged 10 commits into from
Feb 12, 2019

Conversation

colorful-tones
Copy link
Member

@colorful-tones colorful-tones commented Jan 8, 2019

Fixes #11609

Description

These changes are CSS only, and in the Quote block's theme.scss, which is loaded in both the editor and on the front end if theme has opted in to using them. See Gutenberg Handbook: Theme Support.

The CSS affects and/or adds a border to either the left or right depending on the text-align chosen in the editor. Border on text-align: center are removed.

How has this been tested?

Changes were tested in local Docker WP environment with Gutenberg post with all variations of Quote block added. Changes were tested against the TwentyFifteen, TwentySixteen, TwentySeventeen and TwentyNineteen themes.

Tested in Chrome 71.0.3578.98 on macOS 10.14.1.

Screenshots

twentyfifteen-quote-block-gutenberg-style
twentyfifteen-quote-block-style
twentynineteen-quote-block-gutenberg-style
twentynineteen-quote-block-style
twentyseventeen-quote-block-gutenberg-style
twentyseventeen-quote-block-style
twentysixteen-quote-block-gutenberg-style
twentysixteen-quote-block-style

Types of changes

  • New feature (non-breaking change which adds functionality): adds a right border if Quote block is text-align: right. However, this does have some impact on existing editor and/or theme styles for some TwentySomething themes:
    1. TwentyFifteen, TwentySixteen and TwentyNineteen: the editor-blocks.css (Gutenberg editor styles) may need to be updated (see screenshot above). The front end output is fine for TwentyFifteen and TwentySixteen.
    2. TwentyNineteen: the theme's styling will need to be updated, because it will now show both:
      • border-left: 2px solid #0073aa
      • `border-right: 4px solid #000;

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.

@gziolo gziolo added Needs Design Feedback Needs general design feedback. [Block] Quote Affects the Quote Block labels Jan 22, 2019
@jasmussen
Copy link
Contributor

THANK YOU for sticking with this! This seems overall like a very solid pull request, I'm sure we'll get this in relatively quickly. 🏅

This seems to be working on the frontend, but I'm not seeing any changes in the editor. Have you tried a theme that does not provide editor styles and relies on the default vanilla editor stylesheet? I'm not sure why the theme.scss files aren't being picked up. @gziolo any idea?

changes

Styling using this selector, [style="text-align:right"] seems a little funky to me. I mean if it works, and is compatible, it's probably fine. But let's sanity check this with some of the others... @kjellr any insights?

For example for RTL languages, we want to make sure that explicitly set alignments are not autoreversed by PostCSS. For example here's what we have for floats:

	.alignleft {
		/*rtl:ignore*/
		float: left;
		/*rtl:ignore*/
		margin-right: 1em;
	}

	.alignright {
		/*rtl:ignore*/
		float: right;
		/*rtl:ignore*/
		margin-left: 1em;
	}

The /*rtl:ignore*/ comment ensures that the line right after is ignored by the RTL engine that would otherwise automatically rewrite float: left to float: right in the *-rtl.css file. We want these to not be rewritten because when it's an explicit choice by the user to align something right, it doesn't matter which direction it was initially.

Which brings us to the next thing we need to sanity check and be sure of. The blockquote comes with left alignment by default, which is later overrwritten (border-left: none and border-right: something in the right alignment choice). Which means those defaults should be rewritten by the postCSS RTL engine. But we want to make sure whatever is rewritten here, is correctly overwritten when the user makes the aforementioned explicit choice.

In other words, I think what we need to do here is to:

  • Get this to work in the editor
  • Be sure to test every possible alignment, including no alignment, in LTR and RTL text directions both.

Nice work! ✨

@kjellr
Copy link
Contributor

kjellr commented Jan 22, 2019

Styling using this selector, [style="text-align:right"] seems a little funky to me. I mean if it works, and is compatible, it's probably fine. But let's sanity check this with some of the others... @kjellr any insights?

Yeah, I remember running into that recently, too. As far as I know, that's the only way to style these sorts of text alignments within the editor right now — Gutenberg don't provide a special class to hook into for alignment.

As for other notes, I'm in agreement with Joen: This seems solid as long as we can make sure it's working great in the editor (not working for me there either yet), for RTL.

It's worth highlighting again that this may be a small breaking change for some themes. For instance, Twenty Nineteen will need a patch to make sure people don't end up with double borders as noted above.

@jasmussen
Copy link
Contributor

It's worth highlighting again that this may be a small breaking change for some themes. For instance, Twenty Nineteen will need a patch to make sure people don't end up with double borders as noted above.

That's a good point. Is there any way we can achieve the same effect, without having to break things here?

@kjellr
Copy link
Contributor

kjellr commented Jan 22, 2019

Is there any way we can achieve the same effect, without having to break things here?

I'm not sure there is — Twenty Nineteen specifically defines that left border. I'm not sure if other themes will do the same (but I imagine they might?). It would be a fairly simple fix in the theme, but a fix would be needed.

@colorful-tones
Copy link
Member Author

Just wanted to drop a status: I’m off on vacation this entire week. I’ll be back next week and will see if I can schedule this in. I’ll drop another update early next week (Jan. 28). Thanks!

@jasmussen
Copy link
Contributor

No stress, and thanks for your work!

@gziolo
Copy link
Member

gziolo commented Jan 23, 2019

@jasmussen I see some of the styles used in the editor but overridden:

screen shot 2019-01-23 at 18 56 41

screen shot 2019-01-23 at 18 59 40

I don't see styles which use the following selectors:

  • &[style="text-align:right"]
  • &[style="text-align:center"]

@jasmussen
Copy link
Contributor

Indeed, good catch. It seems the editor style needs looser specificity, if possible, to let the theme style more easily override.

@colorful-tones
Copy link
Member Author

Looks like it is taking me a bit longer to circle back on this. I'm hoping in next week or two to get an update on the Pull Request. Thanks for all the testing and feedback!

@gziolo gziolo added this to the 5.2 (Gutenberg) milestone Feb 7, 2019
@m-e-h
Copy link
Member

m-e-h commented Feb 9, 2019

Great idea!!

The reason it's not working is because attribute selectors are kinda funny.

The space between text-align: and right matter. Which is how it's done in the editor HTML.
The front end doesn't have the space, so what you have here should work there since it's an exact match.

You'll probably also need a general * regex thingy when working with the space.

Try this: .wp-block-quote[style*="text-align: right"] for the editor style.

@sarahmonster
Copy link
Member

Thanks so much for sticking this one though, @colorful-tones! It's great to see this implemented. 👏

youknowriad pushed a commit that referenced this pull request Mar 6, 2019
* adjust borders for Quote block text-align

* update attribute selector [style*=“text-align: right”]

* add [style*="text-align:right"] styling for Quote

* add asterisk to [style*=“text-align:center”]

* add [style*="text-align: center"] override as well
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
* adjust borders for Quote block text-align

* update attribute selector [style*=“text-align: right”]

* add [style*="text-align:right"] styling for Quote

* add asterisk to [style*=“text-align:center”]

* add [style*="text-align: center"] override as well
This was referenced Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Quote Affects the Quote Block Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants