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 RTL style overrides, ensure RTL CSS is loaded on RTL sites #2669

Merged
merged 7 commits into from
Jul 19, 2024

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Jul 11, 2024

Fixes #2483. This updates the style.css registration to create an RTL replacement on RTL sites — this means the style-index-rtl.css file will be used instead. This PR also includes a few overrides specific to RTL sites, in the _rtl.scss file.

The issues were broadly with the Meeting Calendar plugin, Sensei, and hardcoded styles on blocks.

  • The meeting calendar could probably be fixed by updating the wp-scripts package there, which would create RTL styles (but the 15 -> 27 version jump would probably cause other issues).
  • Sensei's RTL support is limited, see RTL Design Support Automattic/sensei#1836, so there are overrides in the _rtl.scss file.
  • The Learning Pathway Card sets the padding on the group block directly, and since it's loaded via a template, the convert_inline_style_to_rtl never runs to flip those values (hopefully the the_block_template_html filter will be added in 6.7, which will be a better place to run that filter) — in any case, I've added some CSS to _rtl.scss for that case as well.

Screenshots

Before After
Lesson page Screen Shot 2024-07-11 at 16 50 11
Learning Pathway homepage Screen Shot 2024-07-11 at 16 57 09
Online workshops Screen Shot 2024-07-11 at 16 58 56
Course homepage Screen Shot 2024-07-11 at 17 03 53 Screen Shot 2024-07-11 at 16 59 43
Quiz Screen Shot 2024-07-11 at 17 03 01
@ryelle ryelle self-assigned this Jul 11, 2024
@adamwoodnz
Copy link
Contributor

adamwoodnz commented Jul 11, 2024

The My Courses filter still need to be switched to the opposite side

Screenshot 2024-07-12 at 11 38 54 AM

@adamwoodnz
Copy link
Contributor

Quiz radio inputs look good but checkboxes are misaligned, see https://learn.wordpress.org/quiz/how-to-use-headings-for-accessibility-and-seo/

Screenshot 2024-07-12 at 11 43 57 AM

@adamwoodnz
Copy link
Contributor

adamwoodnz commented Jul 11, 2024

The images in the Learning Pathway cards get cropped from the wrong side on smaller screens (screenshots are at iPad landscape size)

LTR RTL
learn wordpress org_learning-pathways__locale=art_xpirate(iPad) learn wordpress org_learning-pathways_(iPad)

And on the homepage some borders need switching

learn wordpress org__locale=ar(iPad)

Screenshot 2024-07-12 at 11 54 39 AM

Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

Looking much better, but I found a few more things sorry

@ryelle
Copy link
Contributor Author

ryelle commented Jul 15, 2024

The My Courses filter still need to be switched to the opposite side

This looks correct for me:
Screenshot 2024-07-15 at 1 09 31 PM

And on the homepage some borders need switching

I can't replicate this, borders look fine to me:
Screenshot 2024-07-15 at 1 27 15 PM

Do you have an out of date wporg-parent on your sandbox, maybe? These are both things that the RTL filters should fix.

Quizzes with checkboxes have been fixed (6d12c9e)

Screen Shot 2024-07-15 at 13 40 52

And I've flipped the image alignment on learning pathways cards for RTL sites (c53f67c)

Screen Shot 2024-07-15 at 13 41 20

@ryelle ryelle requested a review from adamwoodnz July 17, 2024 14:55
Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

LGTM now except for the border styles. I've updated all on my sandbox and still seeing them. The culprit seems to be https://learn.wordpress.org/wp-content/plugins/gutenberg/build/block-library/common-rtl.css?ver=18.8.0

Screenshot 2024-07-18 at 11 46 24 AM

If you're not seeing it then maybe ignore.

@ryelle
Copy link
Contributor Author

ryelle commented Jul 18, 2024

Aha, I managed to trigger it by preventing inline styles — I was seeing this as an inline style, but it's using the unflipped LTR version here (that seems like a different problem 😬 )

Screenshot 2024-07-18 at 6 03 58 PM

This style should not be flipped in the RTL stylesheet, and is a bug in gutenberg- by setting the border-right-style it inherits default a width & color; and since the left style is not set, it's not visible.

This can be worked around by setting the right style (to make the inline-end border visible) and the left width 0 (to hide the inline-start border). Left/right are flipped in the RTL function in the parent theme, so it's easier to talk in logical property direction 🙂

@ryelle ryelle merged commit 1d7bcbe into trunk Jul 19, 2024
1 check passed
@ryelle ryelle deleted the update/rtl-styles branch July 19, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants