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

Add Text Alignment Support to the Pullquote Block #31996

Closed

Conversation

porada
Copy link

@porada porada commented May 19, 2021

Description

This PR adds support for specifying text alignment inside the Pullquote block using a block toolbar control.

Our users expect the center alignment option to be available (#22784, #31558, Automattic/wp-calypso#52519), because it used to also align the text inside the quote to center. That option was removed probably with the introduction of the Solid Color block style which aligned text to the side. This PR allows the user to overwrite the default alignment defined by any of the available block styles.

Behind the scenes, this PR also adds support for the placeholder attribute in the standard AlignmentToolbar control. This allows the control to reflect the currently used setting without explicitly setting the value.

How has this been tested?

  • Pull the changes.
  • npm run test-unit
  • Open the editor.
  • Insert an instance of the Pullquote block.
  • Set the block style to Solid Color.
  • Use the new toolbar control to overwrite the default text alignment.
  • Make sure the custom text alignment option works intact with any other block alignment and block style option available—both in the editor and on the preview.

Screenshots

Types of changes

New feature

Checklist

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I’ve tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I’ve included developer documentation if appropriate.
  • I’ve updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@porada porada marked this pull request as ready for review May 19, 2021 18:30
@talldan talldan added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label May 20, 2021
@ZebulanStanphill ZebulanStanphill added [Block] Pullquote Affects the Pullquote Block [Type] Enhancement A suggestion for improvement. labels May 20, 2021
@porada
Copy link
Author

porada commented May 20, 2021

Just making a note that the failing check keeps encountering a timeout. The suite should run just fine locally.

Also, please let me know if I should turn 8225d47 into a separate PR.

Copy link
Contributor

@yansern yansern left a comment

Choose a reason for hiding this comment

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

I am testing this on the latest rebased 10.7.1 version.

Unit tests are passing. (There was one failing test unrelated to this PR.)
image

Text alignment on pullquote block works as expected.
image

IMO the codes are clean & excellent.

Comment on lines +34 to +43
/*
* 1. Defining `text-align` on the parent would get overwritten by the theme’s
* defintion of the default style.
* 2. When no text alignment is set, the solid color style aligns text to the
* side, not center.
* 3. Twenty Twenty-One has a hardcoded `text-align` on `blockquote::before`.
* The only way to make the decorative quotation mark follow the same
* alignment as its parent (both in the editor and on the front-end) is
* by resetting the property with `!important`.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Excellent CSS commenting style. 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Plus one to that! 👏🏻

@gziolo gziolo requested review from a team and oandregal August 26, 2021 06:48
@porada
Copy link
Author

porada commented Aug 26, 2021

I can the see the PR has diverged a bit from the trunk over time. Once there is a clear desire to merge this in, I’ll be happy to resolve all the conflicts. Just let me know.

@SiobhyB
Copy link
Contributor

SiobhyB commented Aug 27, 2021

👋 @porada, I did some testing to double-check whether this change has any impact on the iOS and Android apps. I didn't run into issues, but there have been some updates to block settings in the apps in the last few months that aren't included in this PR (more specifically, the introduction of global styles). It'd be great to test again when the PR's updated from trunk, so that we can confirm that all the settings still work as expected on mobile.

Would you be able to give me a ping whenever this PR's updated? I'll follow along with this issue for any updates, too. Thanks!

@Mamaduka
Copy link
Member

Mamaduka commented Sep 7, 2021

Hi, @porada

Can you rebase this branch on top of the current trunk?

Thanks

@Mamaduka Mamaduka added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Oct 14, 2021
@scruffian scruffian mentioned this pull request May 17, 2022
23 tasks
@carolinan
Copy link
Contributor

Text align was added to the block in #30951.

@carolinan carolinan closed this Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Pullquote Affects the Pullquote Block First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants