WordPress.org

Make WordPress Core

Opened 12 months ago

Last modified 2 months ago

#52294 new defect (bug)

Twenty Twenty-One: editor styles are broken in RTL

Reported by: yoavf Owned by:
Milestone: 6.0 Priority: normal
Severity: normal Version: 5.6
Component: Bundled Theme Keywords: has-patch needs-testing
Focuses: rtl Cc:

Description (last modified by yoavf)

The editor-styles in Twenty Twenty-One do not have an RTL version.
This causes major style issues in the editor when in RTL mode.

For example, this is how a nested ordered list looks like:

Notice all items are on the same horizontal level.

This is how it looks with the theme editor styles turned off:

Details of the problem

Currently add_editor_style RTL support is pretty limited. It will, if the file exists, load a style-editor-rtl.css file, in addition to the main style-editor.css file (similarly to how rtl.css is loaded for themes).

The problem with this method is that we don't currently have the tooling to generate that addon rtl file, which only overwrites the necessary CSS declarations.

With the complexity of Twenty Twenty-One style-editor.css file (~2500 lines, SASS generated), creating a manual addon file is not reasonable.

Possible solutions

Any solution should start by generating a fully mirrored style-editor-rtl.css file automatically, using rtlcss.

I considered a couple of possible solutions - and I'd be happy to hear opinions on this:

  1. We could use the Twenty Twenty method in twentytwenty_block_editor_styles - it doesn't use add_editor_style and instead calls wp_enqueue_style, and then uses wp_style_add_data to define that the RTL version should replace the ltr version.
  1. We could update add_editor_style to support loading an additional parameter. I'm attaching an example of how this could be done.

Attachments (3)

method-2-example.diff (3.0 KB) - added by yoavf 12 months ago.
adding a new param to add_editor_style to allow replacing the css file with RTL version
editor-styles-broken.png (54.3 KB) - added by yoavf 12 months ago.
editor-styles-disabled.png (45.6 KB) - added by yoavf 12 months ago.

Download all attachments as: .zip

Change History (21)

@yoavf
12 months ago

adding a new param to add_editor_style to allow replacing the css file with RTL version

#1 @yoavf
12 months ago

  • Version set to 5.6

#2 @yoavf
12 months ago

  • Description modified (diff)

#3 @SergeyBiryukov
12 months ago

  • Component changed from Themes to Bundled Theme

#4 follow-up: @joyously
12 months ago

Another alternative is for the stylesheet to contain both LTR and RTL styles. I do this in my theme. It makes it much easier to keep the styles consistent when they are together.

#5 in reply to: ↑ 4 @yoavf
12 months ago

Replying to joyously:

Another alternative is for the stylesheet to contain both LTR and RTL styles. I do this in my theme. It makes it much easier to keep the styles consistent when they are together.

That works (by targeting the .rtl body class), but do you have a solution to automate the generation of the RTL CSS in the main stylesheet?

#6 @poena
12 months ago

I like the idea of making the new parameter available for all themes.

I searched Trac to see if there were any duplicates. I did not find any but
perhaps it could be paired with #42645, Support passing version number to add_editor_style().

Last edited 12 months ago by SergeyBiryukov (previous) (diff)

This ticket was mentioned in Slack in #themereview by poena. View the logs.


12 months ago

#8 @poena
4 months ago

  • Keywords has-patch needs-testing added
  • Milestone changed from Awaiting Review to 5.9

#9 @sabernhardt
3 months ago

  • Summary changed from Twenty-Twentyone: editor styles are broken in RTL to Twenty Twenty-One: editor styles are broken in RTL

This ticket was mentioned in Slack in #themereview by kafleg. View the logs.


3 months ago

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


3 months ago

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


3 months ago

#13 @chaion07
3 months ago

Hi @yoavf! Thank you so much for reporting this. We discussed this ticket during a recent bug-scrub and based on the discussion, 'Testing Instructions' are to be added. It could also use the keyword 'Add_Editor_Style' (which is currently in proposal stage for the keyword to be officially added) instead of 'Theme'. Additionally, it would be a good idea to add this to the agenda considering the manual test session that the Test Team is scheduled to host tomorrow. After the test report is available, this should be in an ideal position to 'Commit'. Cheers!

Props: @hellofromTonya & @poena

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


3 months ago

This ticket was mentioned in PR #1815 on WordPress/wordpress-develop by pbearne.


3 months ago

  • Keywords has-unit-tests added

Added support for replacing a style sheet with a RTL version in the add_editor_style function

Added rtl editor styles sheets for twentytwentytOne theme
Added tests for add_editor_style

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

#16 @pbearne
3 months ago

  • Keywords has-patch needs-testing has-unit-tests removed

Hi All,

I had to change the code to get the add_editor_style function to get it to work

I have tested that the rtl style sheet loads in the classic editor (tinyMCE)

Good to go

Paul

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


3 months ago

#18 @hellofromTonya
2 months ago

  • Keywords has-patch needs-testing added
  • Milestone changed from 5.9 to 6.0

With 5.9 Beta 1 release happening in a few hours, moving this ticket to 6.0. However, if it can get tested and there's consensus on the patch before the release party, move it back into the milestone.

Note: See TracTickets for help on using tickets.