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

Block library: Refactor Quote block to use class names for text align #16779

Merged
merged 2 commits into from
Aug 8, 2019

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Jul 26, 2019

Description

Related: #16027, #15751, #16777.

This PR follows-up #16035.

Before:

<!-- wp:core/quote {"align":"right"} -->
<blockquote style="text-align:right" class="wp-block-quote">
	<p>Testing deprecated quote block...</p>
	<cite>...with a caption</cite>
</blockquote>
<!-- /wp:core/quote -->

After:

<!-- wp:quote {"align":"right"} -->
<blockquote class="wp-block-quote has-text-align-right"><p>Testing deprecated quote block...</p><cite>...with a caption</cite></blockquote>
<!-- /wp:quote -->

How has this been tested?

Using one of the existing branches (master probably) add a few Quote blocks and set different text alignments and save your post. Open the same post with this branch and ensure that it still works

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 [Type] Enhancement A suggestion for improvement. [Package] Block library /packages/block-library [Block] Verse Affects the Verse block labels Jul 26, 2019
@gziolo gziolo self-assigned this Jul 26, 2019
@youknowriad youknowriad requested a review from senadir July 29, 2019 09:27
@draganescu
Copy link
Contributor

With this change on there is a small inconsistency with master, not sure if intended or not, when text is aligned centre the left border still shows, while on master it doesn't:

This change:

Screenshot 2019-07-31 at 15 48 26

Master:

Screenshot 2019-07-31 at 15 47 32

@gziolo gziolo force-pushed the update/quote-text-align branch from da1ad4f to 71d2c7f Compare July 31, 2019 13:37
@gziolo gziolo added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Jul 31, 2019
@gziolo
Copy link
Member Author

gziolo commented Jul 31, 2019

With this change on there is a small inconsistency with master, not sure if intended or not, when text is aligned centre the left border still shows, while on master it doesn't:

I partially fixed with 71d2c7f.

This blue thing comes from Twenty Nineteen theme:

Screen Shot 2019-07-31 at 15 42 16

I marked this PR as Needs Dev Note as we will need to let theme developers know about this change which needs to happen to make blocks which text alignments easier to style. At the moment they use the very awkward syntax as you can see in 71d2c7f.

@gziolo gziolo requested review from kjellr, mapk and karmatosed July 31, 2019 13:45
@gziolo gziolo added [Block] Quote Affects the Quote Block and removed [Block] Verse Affects the Verse block labels Jul 31, 2019
@kjellr
Copy link
Contributor

kjellr commented Jul 31, 2019

This blue thing comes from Twenty Nineteen theme:
...
I marked this PR as Needs Dev Note as we will need to let theme developers know about this change which needs to happen to make blocks which text alignments easier to style. At the moment they use the very awkward syntax as you can see in 71d2c7f.

I'll get a patch ready for Twenty Nineteen to be compatible with this change. 👍

@kjellr
Copy link
Contributor

kjellr commented Jul 31, 2019

Actually, I'm not seeing that blue line issue. 🤔 I don't think the latest version of Twenty Nineteen actually does provide the style you noted above. It provides a border-width and a border-color, but not a position, and then piggybacks off the default styles provided from Gutenberg.

https://themes.trac.wordpress.org/browser/twentynineteen/1.4/style-editor.scss#L393

... and here's how that looks in the editor when I try it out:

Screen Shot 2019-07-31 at 10 55 04 AM

@gziolo gziolo force-pushed the update/quote-text-align branch from 71d2c7f to b8d1b0b Compare August 8, 2019 09:52
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@gziolo gziolo merged commit 509c9cb into master Aug 8, 2019
@gziolo gziolo deleted the update/quote-text-align branch August 8, 2019 12:49
@gziolo gziolo added this to the Gutenberg 6.3 milestone Aug 8, 2019
gziolo added a commit that referenced this pull request Aug 29, 2019
…#16779)

* Block library: Refactor Quote block to use class names for text align

* Refactor CSS theme styles to use new text align classes
gziolo added a commit that referenced this pull request Aug 29, 2019
…#16779)

* Block library: Refactor Quote block to use class names for text align

* Refactor CSS theme styles to use new text align classes
@youknowriad youknowriad removed the Needs Dev Note Requires a developer note for a major WordPress release cycle label Sep 24, 2019
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 [Package] Block library /packages/block-library [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants