-
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
FontSizePicker: Fix header order in RTL languages #44590
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.
Thank you @noisysocks for this fix!
Another way to test for these changes is directly in Storybook — we have added a LTR/RTL switcher, so that you don't necessarily have to change the WordPress language settings :)
Kapture.2022-09-30.at.10.18.03.mp4
In this case, I noticed that the RTL styles are not applied when the component dynamically updates from LRT to RTL at runtime.
I think this is due some how the fact that a change in writing direction doesn't force the styled
component to re-render (and therefore recompute styles). One approach could be to use rtl.watch()
and trigger a re-render, but for such a simple scenario probably the better solution is to use a custom flexbox layout, which completely avoids the need for the rtl
function, and therefore should also update dynamically when the writing direction changes at runtime!
Something like this maybe?
diff --git a/packages/components/src/font-size-picker/styles.ts b/packages/components/src/font-size-picker/styles.ts
index 86cdbc08c6..afcdde1b29 100644
--- a/packages/components/src/font-size-picker/styles.ts
+++ b/packages/components/src/font-size-picker/styles.ts
@@ -19,14 +19,14 @@ export const Container = styled.fieldset`
`;
export const HeaderLabel = styled( BaseControl.VisualLabel )`
- display: inline-block;
+ display: flex;
+ gap: ${ space( 1 ) };
+ justify-content: flex-start;
margin-bottom: 0;
`;
export const HeaderHint = styled.span`
color: ${ COLORS.gray[ 700 ] };
- display: inline-block;
- ${ rtl( { marginLeft: space( 1 ) }, { marginRight: space( 1 ) } )() }
`;
export const Controls = styled.div< {
Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>
Hey @ciampo. I applied your suggestion—works a treat! 🙂 |
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.
Neat 🚀
Thank you!
What?
Addresses a bug noticed by @ciampo in #44483 (review) where the Size label and hint appear in the wrong order in RTL languages.
Why?
How?
Setting
display: inline-block
makes the browser re-order the elements automatically becausedirection: rtl
will apply to all noninline
elements. Settingmargin-right
instead ofmargin-left
when the language is RTL ensures that the margin goes on the correct side of the hint.Testing Instructions
Best to test this in the real app as the RTL setting in Storybook doesn't affect the result of
wp.i18n.isRTL()
.Screenshots or screencast