-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Improve RTL rendering #3831
Improve RTL rendering #3831
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.
Not familiar with the filter property, seems it looks ok.
However, I'm getting a PHP warning when I switch to RTL.
<b>Warning</b>: strpos() expects parameter 1 to be string, array given in
<b>/srv/www/wordpress-develop/public_html/build/wp-includes/functions.php</b> on line <b>3451</b><br />
Seems when Gutenberg is activated, $mce_init['plugins']
in the core _mce_set_direction()
is no longer a string but an array. When Gutenberg is deactivated, it's a string, as expected.
I haven't tested yet, but by looking at the screenshot, I think we should switch the "undo/redo" icons in RTL |
Great feedback, both — I'll look at how to best flip around undo/redo, though I think I'll need help fixing that PHP error. I assumed it was an issue with my local docker. |
I pushed a fix for the php error. About the styling in RTL. I saw a lot of places where we need to change padding/marging from left to right etc... I'm probably missing some other places as well, I wonder if it's not better to just use something like https://www.npmjs.com/package/postcss-rtl |
Thanks so much Riad.
I think we absolutely should! Do you have time to help me out with this? It's not super urgent. |
@jasmussen Sure let me try it :) |
🏅 |
c842d99
to
a060799
Compare
Update with the postcss plugin, it looks like it works properly. We still have some issues (popovers) because these are JS generated styles but most of the issues are solved. What do you think? |
I think this is incredibly impressive, and thanks so much for doing this. I think it's a solid update, and brings us to a point where we can have individual highly specific tickets for each remaining RTL item, instead of one big mammoth umbrella "fix RTL" ticket. 👍 👍 and thanks again |
@jasmussen It looks like I broke non-rtl languages :) looking |
WP adds a There's also a small priority issue I had to fix for buttons. |
#3844 is merged, let's close this one. Good job everyone |
🎉🎉 |
Description
This PR improves the RTL appearance of Gutenberg:
Additionally, it might fix #1565, as it also flips horizontally the arrow icon.
How Has This Been Tested?
Chrome on Mac.
Steps to test