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

Add initial setup for RTL support #351

Merged
merged 24 commits into from
Mar 9, 2020
Merged

Conversation

swissspidy
Copy link
Collaborator

@swissspidy swissspidy commented Feb 19, 2020

Since styled components now supports this more or less out of the box, adding RTL support was rather trivial.

Fixes #55.
Fixes #317.

To-do

Before this PR:

Screenshot 2020-03-09 at 11 26 33

After this PR:

Screenshot 2020-03-09 at 11 36 55

@swissspidy swissspidy changed the title First pass at RTL support [WIP] RTL support Feb 20, 2020
@swissspidy
Copy link
Collaborator Author

Still early, and not a priority, just curious to hear some thoughts about where the issues might be.

How to test

  1. Check out this branch, build
  2. Option A: Change your WordPress site's language to a RTL locale like Arabic
  3. Option B (easier): Install the RTL Tester plugin (search for RTL Tester on the plugins screen) and use the new RTL Tester switch in the admin toolbar to switch back and forth
  4. Drag around elements on the screen

Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

Do you think we should add rtl-tester plugin to the build to make it easier to test?

@swissspidy
Copy link
Collaborator Author

@spacedmonkey Do you mean adding RTL Tester plugin to the test environment? Or the docker setup?

I was also looking into storybook support with https://www.npmjs.com/package/storybook-addon-rtl

@spacedmonkey
Copy link
Contributor

Do you mean adding RTL Tester plugin to the test environment? Or the docker setup?

Both I guess. I originally meant docker setup, but testing environment makes sense too.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 25, 2020

Size Change: +2.27 kB (0%)

Total Size: 346 kB

Filename Size Change
assets/js/edit-story.js 305 kB +2.27 kB (0%)
ℹ️ View Unchanged
Filename Size Change
assets/css/edit-story.css 189 B 0 B
assets/css/stories-dashboard.css 165 B 0 B
assets/js/stories-dashboard.js 40.9 kB 0 B

compressed-size-action

@spacedmonkey spacedmonkey linked an issue Feb 25, 2020 that may be closed by this pull request
@spacedmonkey
Copy link
Contributor

spacedmonkey commented Feb 25, 2020

Screenshot 2020-02-25 at 14 52 12

Are these icons undo / redo the right way around?

@spacedmonkey
Copy link
Contributor

image
Screenshot 2020-02-25 at 14 58 20

Insert element is broken.

@swissspidy
Copy link
Collaborator Author

Are these icons undo / redo the right way around?

I first had this as a to-do, but realized Gutenberg displays them the same way, so it seems to be correct 🤷‍♂

Insert element is broken.

Yeah I think that's the same issue as in the screenshot at the top. Not sure what exactly is the culprit though. Have yet to check it out.

assets/src/edit-story/components/canvas/carousel/index.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/canvas/pagenav/index.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/canvas/pagenav/index.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/canvas/useCanvasKeys.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/canvas/useCanvasKeys.js Outdated Show resolved Hide resolved
@swissspidy swissspidy added the Type: Enhancement New feature or improvement of an existing feature label Feb 26, 2020
@swissspidy
Copy link
Collaborator Author

So far Movable seems to be the biggest remaining issue here.

@swissspidy
Copy link
Collaborator Author

@miina Perhaps you have some ideas on the Movable part (not urgent)

@swissspidy swissspidy marked this pull request as ready for review March 6, 2020 09:42
@swissspidy swissspidy requested a review from pbakaus as a code owner March 6, 2020 09:42
@swissspidy swissspidy dismissed spacedmonkey’s stale review March 6, 2020 12:06

Changes have been applied; many changes since then

@swissspidy swissspidy changed the title [WIP] RTL support Add initial setup for RTL support Mar 9, 2020
Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

Everything looking good in my testing. The only thing I found was the dragabout elements, as noted here.

Fix Movable issues when dragging elements on a "flipped" canvas -> could be in new PR

Going to loop @miina and @dvoytenko for this one. Feels like it is a simple inversion of axioses, but I could be wrong.

@swissspidy swissspidy dismissed dvoytenko’s stale review March 9, 2020 12:14

Plenty of changes since initial review

@swissspidy swissspidy merged commit 10cae2d into master Mar 9, 2020
@swissspidy swissspidy deleted the add/styled-components-rtl branch March 9, 2020 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internationalization (I18N) Type: Enhancement New feature or improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Double-check PostCSS config Baseline RTL Support
4 participants