Skip to content

Blocks: Add direction attribute / LTR button to paragraph block#10663

Merged
gziolo merged 1 commit intomasterfrom
add/p-direction
Oct 18, 2018
Merged

Blocks: Add direction attribute / LTR button to paragraph block#10663
gziolo merged 1 commit intomasterfrom
add/p-direction

Conversation

@aduth
Copy link
Copy Markdown
Member

@aduth aduth commented Oct 16, 2018

Closes #138

This pull request seeks to add an LTR toggle button to the paragraph block toolbar when an RTL locale is active.

image

It adds a new direction block to the paragraph block. As implemented, the value will only ever be ltr or undefined, given current UI behavior as toggle only shown when the editor is configured as RTL. When non-undefined, it assigns a dir attribute to the saved markup. This should be entirely backward-compatible with existing paragraph blocks.

Future compatibility notes:

  • Directionality could be abstracted as a common blocks feature, similar to align, color, etc.

Testing Instructions:

Verify there's no LTR button when an LTR locale is active.

Verify that an LTR button is active and serves as a toggle for paragraph block when an RTL locale is active.

cc @yoavf @aahani

@aduth aduth added the [Feature] Blocks Overall functionality of blocks label Oct 16, 2018
@aduth aduth added this to the 4.1 - UI freeze milestone Oct 16, 2018
@aduth aduth requested a review from youknowriad October 16, 2018 19:47
},
direction: {
type: 'string',
enum: [ 'ltr', 'rtl' ],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we support this enum property?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not yet 😉

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Well, to clarify, it's supported in the server-side validation, but isn't ported to the client. Idea with adding here now is it "just works" when ported.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's probably a lot of block attributes where this could be considered. align for instance

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
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.

The approach looks good to me. I wonder if we should add an RTL e2e test (not just specific to this though). I'd appreciate a review from @yoavf as he has more experience on these matters.

controls={ [
{
icon: 'editor-ltr',
title: _x( 'Left to right', 'editor button' ),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

In case this raises eyebrows, the hope / intention is to inherit the existing string:

https://github.com/WordPress/WordPress/blob/da7a80d67fea29c2badfc538bfc01c8a585f0cbe/wp-includes/class-wp-editor.php#L1128

@gziolo
Copy link
Copy Markdown
Member

gziolo commented Oct 18, 2018

Let's merge it as is and revisit later if @yoavf has some concerns.

@gziolo gziolo merged commit 70583ac into master Oct 18, 2018
@gziolo gziolo deleted the add/p-direction branch October 18, 2018 07:41
@mtias mtias added the Internationalization (i18n) Issues or PRs related to internationalization efforts label Oct 18, 2018
antpb pushed a commit to antpb/gutenberg that referenced this pull request Oct 26, 2018
@woorise
Copy link
Copy Markdown

woorise commented Sep 25, 2019

Is it possible to manually add/enable the direction attribute / LTR button if the current locale isn't RTL?

@dirad
Copy link
Copy Markdown

dirad commented Oct 28, 2020

Is it possible to manually add/enable the direction attribute / LTR button if the current locale isn't RTL?

We need this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Blocks Overall functionality of blocks Internationalization (i18n) Issues or PRs related to internationalization efforts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for RTL scripts

6 participants