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

[Master feature] Support RTL for amp-story #11647

Closed
9 tasks done
newmuis opened this issue Oct 11, 2017 · 4 comments
Closed
9 tasks done

[Master feature] Support RTL for amp-story #11647

newmuis opened this issue Oct 11, 2017 · 4 comments

Comments

@newmuis
Copy link
Task lists! Give feedback
Contributor

@newmuis newmuis commented Oct 11, 2017

Right now, everything is designed as LTR. This is a cover bug for enabling RTL support. We should consider the story to be RTL if:

  1. Any ancestor of the story element has its dir attribute set to "rtl"
  2. Failing that, the direction property of its computed style is "rtl"
  3. Failing that, document.dir is set to "rtl"

If none of these are true, we should consider the story as LTR. We can calculate this once per story, at build time.

  • Navigation areas of the screen should be flipped (i.e. tapping the left side of the screen will advance to the next page; the right side of the screen will go back to previous)
  • Keyboard keys should be flipped (left arrow for next; right arrow for previous)
  • System layer UI elements should be flipped (the buttons normally in the top right should be in the top left, and vice versa), on desktop and mobile
  • Bookend UI elements should be flipped
  • amp-story-consent UI elements should be flipped
  • Share menu UI elements should be flipped
  • Info dialog UI elements should be flipped
  • Hint layer UI elements should be flipped
  • Any icons indicating directionality based on reading direction (as opposed to, say, handedness) should be flipped
@aghassemi
Copy link
Contributor

@aghassemi aghassemi commented Apr 25, 2018

@newmuis @choumx Could we consider move this to P1/P2? Another market this has come up is Japan.

@morsssss
Copy link
Contributor

@morsssss morsssss commented Apr 25, 2018

This is harder no doubt... but would it be worth creating another issue to enable vertical instead of horizontal? At least one major language group expects that :)

@newmuis newmuis assigned Enriqe and unassigned newmuis May 11, 2018
@newmuis newmuis moved this from Fixit / Cleanup to New Features in Stories - By Type Jun 5, 2018
@newmuis newmuis changed the title Support RTL for amp-story [Master feature] Support RTL for amp-story Jul 2, 2018
@newmuis newmuis added this to Feature Backlog in AMP HTML Project Roadmap via automation Jul 2, 2018
@newmuis newmuis moved this from New Features to Master Features in Stories - By Type Jul 3, 2018
@newmuis newmuis added this to To do in Stories - Roadmap [DEPRECATED] via automation Jul 3, 2018
@newmuis newmuis removed this from Master Features in Stories - By Type Jul 3, 2018
@newmuis newmuis moved this from Not started to In Progress in Stories - Roadmap [DEPRECATED] Jul 3, 2018
@newmuis
Copy link
Contributor Author

@newmuis newmuis commented Jul 27, 2018

The visual diff tests added in #17110 show that many of the templated UIs need some love in RTL:

issue_rtl_bookend
issue_rtl_consent
issue_rtl_info_dialog

The share menu actually looks okay:

rtl_share_menu

@Enriqe
Copy link
Contributor

@Enriqe Enriqe commented Jul 27, 2018

Yep, the share menu was just fixed in #17098 alongside the system layer. I am working on the remaining templated UIs. Thanks for including RTL in the visual diffs!

AMP HTML Project Roadmap automation moved this from Next Up to Shipped Aug 8, 2018
Stories - Roadmap [DEPRECATED] automation moved this from In Progress to Done Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants