Amp-wp: Fix RTL styling in Stories editor

Created on 12 Aug 2019  路  13Comments  路  Source: ampproject/amp-wp

Bug Description

The Stories editor is currently not usable at all when using an RTL language. Even with the changes in #2977 applied.

It's mostly arrow icons that are not flipped, and the transform in the EditorCarousel component that doesn't change when isRTL.

Expected Behaviour

The stories editor should work just fine in RTL mode.

Steps to reproduce

  1. Install RTL Tester plugin and click on "Switch to RTL" in the admin bar. _Or_, alternatively, switch user language to an RTL language like Arabic.
  2. Open stories editor
  3. Notice how not all of the layout elements are "flipped" and the editor is not usable
  4. Notice that the context menu doesn't seem to be working either.

Screenshots

Screenshot 2019-08-12 at 15 15 46

Additional context

  • WordPress version: 5.3
  • Plugin version: current develop
  • Gutenberg plugin version (if applicable): 6.6

_Do not alter or remove anything below. The following sections will be managed by moderators only._

Acceptance criteria

  • [x] The current page in the carousel should be correctly centered and navigation should be possible the same way as in LTR mode
  • [x] Iconography should be adapted (flipped) accordingly in the context menu as well as the carousel navigation
  • ~[ ] It should be possible to access the context menu as expected~ -> see #3607
  • [x] Dragging between pages should work as expected and highlight the correct target pages
  • [x] Resize handles should be displayed the same way as in LTR mode

Implementation brief

  • Wherever code changes are needed, select( 'core/block-editor' ).getSettings().isRTL is used to switch the logic for RTL mode.
  • If necessary, CSS changes are made to improve RTL compatibility.
  • Add rtl:ignore comments for resize handles

QA testing instructions

Setup before any test below

  1. Activate RTL Tester plugin
  2. Click on "Switch to RTL" in the admin toolbar at the top
  3. Create a new story

Verify carousel

  1. Create few pages and go through the carousel
  2. Verify: the currently selected page should always be shown in the center
  3. Verify: carousel arrows should point into the right direction and navigate correctly
  4. Verify: dragging pages in carousel works as intended

Verify resize handles

  1. Edit text block
  2. Verify: the resize handles should be displayed at every location correctly

Verify dragging across pages

  1. Drag block to another page
  2. Verify: the correct page is highlighted when hovering over it
  3. Verify: the correct placement when dropped on another place

Verify CTA block layout

  1. Add CTA block on a new page
  2. Click on button to edit the URL
  3. Verify: the URL input field should not overlap the button

Demo

Changelog entry

  • Improved RTL support in the editor
AMP Stories (obsolete) Accessibility Bug i18n

All 13 comments

@swissspidy Can you provide information on how to setup development so you can swtich rtl as you have done in the example above.

Acceptance criteria should highlight the places that need to be fixed.

Will do 馃憤

@spacedmonkey Added steps to reproduce as well as more detailed ACs.

Bugs as I see them

  • Right click + alt + f10 doesn't show context menu.
  • Navigation arrows incorrect.
  • Attachment cross in on the wrong side.
  • Alignment of pages doesn't go RTL
  • Dragging between pages doesn't work.
  • After 3 / 4 pages inserted layout completely breaks.
  • Button, insert link overlay button.

These are all the issues I flagged on my first pass. All these bugs are pretty big. I think each of these issues should be broken into they own issue with a new support of RTL support.

  • Dragging between pages doesn't work.
  • After 3 / 4 pages inserted layout completely breaks.
  • Navigation arrows incorrect.

Those are the most pressing issues at the moment as the editor is otherwise unusable. Fixing those was rather easy, I just created a quick PoC PR for testing that: https://github.com/ampproject/amp-wp/pull/3558

  • Attachment cross in on the wrong side.

The close icon is on the correct side. The layout in the editor and on the frontend is identical. Note that for RTL languages everything is flipped.

  • Alignment of pages doesn't go RTL

Can you elaborate on that?

  • Button, insert link overlay button.

Seems like a pretty small thing to fix in CSS.

  • Right click + alt + f10 doesn't show context menu.

That one seems like it might take longer to resolve, but exploring that would be part of the IB.

  • Alignment of pages doesn't go RTL

Screenshot from 2019-10-22 16-21-27

With only one page, the first one is completely off the edge of the screen and not visible or usable.

Screenshot from 2019-10-22 16-21-42

With 2 pages, second page is visible, just page not accessible or usable. Second page is partly cover by navigation.

Screenshot from 2019-10-22 16-21-56

With 3 pages, more usable area. Other two blocks are more visible. Left side of block is cut off and no border visible

Screenshot from 2019-10-22 16-42-09

With 4 pages, the 4th and current page not visible. Bottom navigations jumps left into wrong position.

Screenshot from 2019-10-22 16-42-25

With 5 pages, bottom navigations disappears. Top buttons jump to middle. Current page not visible.

Screenshot from 2019-10-22 16-42-39

With 6 pages, bottom navigations disappears. Top buttons disappears. Current page not visible.

  • Button, insert link overlay button.
    Screenshot from 2019-10-22 16-35-03

  • Right click + alt + f10 doesn't show context menu.
    Not working in Ubuntu. Might want to check different browsers and OSes on this one.

If we are going to support RTL going forward, this will need to be part of the e2e and all issues will need to be QAed against it.

Looping @csossi for his thought. Might be worth him reviewing the story editor and seeing if he can spot anymore bugs.

Alignment of pages doesn't go RTL

Gotcha. Thought you meant something different. This layout issue is fixed in my PR.

Agreed on testing part. Note that I have yet to add the RTL Tester plugin to the QA environment.

Note that I have yet to add the RTL Tester plugin to the QA environment.

The plugin is available there now.

After reviewing #3558 , I noticed that most of the mega issues I flagged around the page alignment were not fixed. These issues are P0, as the story editor is unusable.

Also the latest PR, make a different bug, where the arrow are going the wrong way. I will open another PR to try and fix some of the issues.

More testing, I found the bug fixed.

Verified in QA

Was this page helpful?
0 / 5 - 0 ratings

Related issues

schlessera picture schlessera  路  5Comments

swissspidy picture swissspidy  路  4Comments

swissspidy picture swissspidy  路  3Comments

westonruter picture westonruter  路  5Comments

westonruter picture westonruter  路  3Comments