Make WordPress Core

Opened 11 years ago

Last modified 5 months ago

#31798 new defect (bug)

Editor: switch undo and redo in RTL

Reported by: yoavf's profile yoavf Owned by:
Milestone: Future Release Priority: low
Severity: normal Version:
Component: Editor Keywords: has-patch has-test-info
Focuses: rtl Cc:

Description

The undo and redo icons in RTL should we switched.

LTR:
https://cloudup.com/cHOxlUfHbAp

Current RTL:
https://cloudup.com/cizBcGBMkCa

Proper RTL:
https://cloudup.com/cSCyiF_HxMA

Attachments (3)

31798.diff (416 bytes) - added by yoavf 11 years ago.
31798-gruntfile.diff (902 bytes) - added by yoavf 11 years ago.
31798.patch (1.5 KB) - added by iseulde 11 years ago.

Download all attachments as: .zip

Change History (21)

@yoavf
11 years ago

#1 follow-up: @ocean90
11 years ago

  • Component changed from Press This to Editor
  • Priority changed from normal to low
  • Summary changed from Press This: switch undo and redo in RTL to Editor: switch undo and redo in RTL
  • Version trunk deleted

This is not specific to Press This, rather Editor in all.

Related chats on Slack:

Maybe we should extend our swap-dashicons-left-right-arrows rule with these icons: trunk/Gruntfile.js@31875#L196

Last edited 11 years ago by ocean90 (previous) (diff)

#2 @yoavf
11 years ago

Thanks for reminding me of these conversations @ocean90. Indeed, this affects the general editor too.

To recap my personal opinion on icons in rtl: they don't necessarily need to be flipped, unless they represent some kind of directionality. In this case, they do.

#3 in reply to: ↑ 1 @yoavf
11 years ago

Replying to ocean90:

Maybe we should extend our swap-dashicons-left-right-arrows rule with these icons: trunk/Gruntfile.js@31875#L196

Oh nice, I never saw that beore (that's what I get for sitting out of dev cycles).

Anyway, the regex in swap-dashicons-left-right-arrows expects the values to be surrounded by double quotes, and we have (in this file, and probably other) content values surrounded by single quotes.

Should we improve the regex to catch both, or do we have a guideline on how to surround content values this way or another?

Version 0, edited 11 years ago by yoavf (next)

#4 @yoavf
11 years ago

31798-gruntfile.diff does this via swap-dashicons-left-right-arrows. As a side effect, single quotes will be replaced by double quotes in content properties, in the rtl css.

@iseulde
11 years ago

This ticket was mentioned in Slack in #core-editor by iseulde. View the logs.


11 years ago

#7 @azaozz
11 years ago

  • Milestone changed from 4.2 to Future Release

I'm actually not sure if this will be more beneficial than annoying. Swapping buttons will definitely be annoying, the users will have to re-learn them. This sounds a bit like swapping right and left.

Also, is there a consensus: in all countries with RTL languages, which is considered to mean "forward", "right" or "left"? Perhaps some more input from people that are native RTL language speakers will be beneficial.

#8 @louyx
11 years ago

Speaking of Arabs, and I think this applies to all RTL languages speaker, in books you have left for forwards and right for backwards.

Also the UI is flipped in RTL so if you consider the page starts by the sidebar and moves in the other direction, it'll be on the right, and forwards will be to the left.

#9 follow-up: @iseulde
10 years ago

  • Keywords reporter-feedback added

@yoavf, any feedback?

#10 in reply to: ↑ 9 @yoavf
10 years ago

Replying to iseulde:

@yoavf, any feedback?

I still think we should switch - haven't tested your patch yet, will try to do that soon.

#11 @swissspidy
9 years ago

  • Keywords needs-testing added; reporter-feedback removed
  • Milestone changed from Future Release to 4.8

This ticket was mentioned in Slack in #core by flixos90. View the logs.


9 years ago

#13 @flixos90
9 years ago

  • Milestone changed from 4.8 to Future Release

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


4 years ago

This ticket was mentioned in PR #9098 on WordPress/wordpress-develop by @SirLouen.


5 months ago
#15

This is a refresh for Refresh 31798.patch
More testing below

Trac ticket: https://core.trac.wordpress.org/ticket/31798

#16 @SirLouen
5 months ago

  • Keywords has-test-info added; needs-testing removed

Test Report

Description

✅ This report validates that the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/9098.diff

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.29.0
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 137.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-One 2.5
  • MU Plugins: None activated
  • Plugins:
    • Classic Editor 1.6.7
    • Test Reports 1.2.0
    • Press This 1.1.2

Testing Instructions

  1. This can be tested either with Classic Editor or with the Press This Editor, install any of these two
  2. Switch to an RTL language like العربية
  3. Now open the editor, whichever you have installed
  4. 🐞 The undo and redo buttons are reversed

Additional Notes

  • Despite this is affecting unsupported components, this is something defined in Gruntfile which could affect any related component in the future. The solution by @ocean90 was adding the f171 and f172 (undo and redo) to the Gruntfile plugin. But also, with the map alternative is way WETter not having to add conditionals for both directions.
  • This is a nice little patch that could be great for RTL users, despite it is aiming for slightly old components.

Actual Results

  • ✅ Issue resolved with patch.

Supplemental Artifacts

Before patch:
https://i.imgur.com/or1nFPr.png
https://i.imgur.com/GyyDdpr.png

After patch:

https://i.imgur.com/3lEg00z.png
https://i.imgur.com/HJvOqq8.png

Last edited 5 months ago by SirLouen (previous) (diff)

This ticket was mentioned in Slack in #core-test by sirlouen. View the logs.


5 months ago

#18 @kush123
5 months ago

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/9098.diff

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 135.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty 2.9
  • MU Plugins: None activated
  • Plugins:
    • Classic Editor 1.6.7
    • Test Reports 1.2.0

Actual Results

  1. ✅ Issue resolved with patch when followed the testing instructions provided.

Supplemental Artifacts

After Patch: https://i.ibb.co/WNFGGpTh/Screenshot-2025-06-30-at-10-38-07-PM.png

Note: See TracTickets for help on using tickets.