Skip to content
This repository was archived by the owner on Jul 12, 2024. It is now read-only.

Load RTL CSS files #1781

Merged
merged 4 commits into from
Mar 14, 2019
Merged

Load RTL CSS files #1781

merged 4 commits into from
Mar 14, 2019

Conversation

Aljullu
Copy link
Contributor

@Aljullu Aljullu commented Mar 12, 2019

Fixes most issues (but not all) from #1776.

This PR:

  • Installs webpack-rtl-plugin so we generate an automated RTL CSS file. It uses a fork of the plugin, as it's done in Gutenberg (Add automated RTL support WordPress/gutenberg#3844).
  • Loads RTL CSS files when using an RTL locale.
  • Removes legacy RTL styles from our current CSS.

Screenshots

image

Detailed test instructions:

  • Install an RTL language in your WordPress, for example, Arabic.
  • Go to Settings and change the default language to that one.
  • Press Ctrl+U to view the page source and verify wc-admin CSS files contain rtl in their name.
  • If you want, you can take a look at RTL issues #1776 and verify most issues are gone! 🔥

@Aljullu Aljullu self-assigned this Mar 12, 2019
@Aljullu Aljullu requested a review from a team March 12, 2019 18:54
@probot-autolabeler probot-autolabeler bot added focus: components Issues for woocommerce components Packages labels Mar 12, 2019
@Aljullu Aljullu marked this pull request as ready for review March 13, 2019 10:09
@Aljullu Aljullu mentioned this pull request Mar 13, 2019
18 tasks
Copy link
Collaborator

@psealock psealock left a comment

Choose a reason for hiding this comment

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

Nice work with this one! I just checked against issues in #1776 marked with this PR (1781) and its looking great. Thanks for handling these cases.

Just one comment about targeting the develop branch.

@@ -106,7 +106,8 @@
"stylelint": "9.9.0",
"stylelint-config-wordpress": "13.1.0",
"webpack": "4.28.3",
"webpack-cli": "3.1.2"
"webpack-cli": "3.1.2",
"webpack-rtl-plugin": "github:yoavf/webpack-rtl-plugin#develop"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we target master ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I chose that one to keep consistency with Gutenberg:
https://github.com/WordPress/gutenberg/blob/master/package.json#L123

Ideally, we would be using webpack-rtl-plugin from NPM, but we need this PR to be merged upstream before: romainberger/webpack-rtl-plugin#15.

@Aljullu Aljullu merged commit e48685e into master Mar 14, 2019
@Aljullu Aljullu deleted the fix/load-rtl-css branch March 14, 2019 10:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants