-
Notifications
You must be signed in to change notification settings - Fork 144
Conversation
There was a problem hiding this 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" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we target master
?
There was a problem hiding this comment.
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.
Fixes most issues (but not all) from #1776.
This PR:
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).Screenshots
Detailed test instructions:
rtl
in their name.