feat(styles): convert styles to CSS logical properties, enhance RTL support#14580
Conversation
6ec1cda to
9d78e2f
Compare
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
7d119a2 to
00ab59c
Compare
|
Seems like the only LTR Percy change is due to the addition of padding on |
alisonjoseph
left a comment
There was a problem hiding this comment.
This all looks good to me! 💪
There was a problem hiding this comment.
Hey @tw15egan
I went throught all the components to see if I could spot something that might be missing, not sure if all the points I made are valids, but maybe it's worth taking a look!
I'm happy to help on these if needed!
Btw, awesome change in the storybook with the RTL mode!! 🚀🔥
- Checkbox - Skeleton example
When using the margin in this format: margin: 1px 1px 1px 1px the values don’t get translate to “RTL”. Maybe we won't be able to use this method to write margin, padding, etc...
- CodeSnippet - Multiline, Singleline
Horizontal scrollbar getting a blur in the left and right side
- Pagination
Those arrow should change directions? I search a little bit and found these examples:
https://github.com/mui/mui-x/issues/8665
https://bootstrap.rtlcss.com/docs/4.0/components/pagination/#working-with-icons
From my understanding it shouldn’t, or am I wrong? Take PaginationNav as an example maybe. There the arrows kept the directions
- ProgressBar
The ProgressBar should switch the side where it starts or kept the same side (left)
- Slider
Seems like the Slider is getting the wrong value, maybe a logic would have to be written to switch the max and min values when in RTL mode (idk if we can track that in JS).
tay1orjones
left a comment
There was a problem hiding this comment.
Overall looks great to me. I agree with Gui's comments above, though I think we could address those in separate PRs if you'd like. The diff on this one is pretty meaty as-is 😅
| // Need to set random key to recalculate Popover coordinates | ||
| setRandomKey(Math.floor(Math.random() * 10)); |
There was a problem hiding this comment.
Oh wow great call, I hadn't thought about the need to re-render in some instances like this.
There was a problem hiding this comment.
Thanks to your suggestions as a way to rerender components I thought of trying it, and it worked perfectly!
| menu.current.style.insetInlineStart = `initial`; | ||
| menu.current.style.insetInlineEnd = `${pos[0]}px`; |
There was a problem hiding this comment.
I'm so glad these work with CSSOM like this 🙌
There was a problem hiding this comment.
Yeah, I was super happy this worked, since this seemed to resolve most of the RTL issues with Menu. I had only been looking at the stylesheets!
…rt (#14531) * refactor(styles): update styles to use CSS logical properties * fix(styles): change padding-inline-end to padding-inline * fix(styles): run stylelint --fix on components * fix(styles): run stylelint --fix on utilities * fix(Select): test padding fix * chore(stylelint): run styelint on files outside @carbon/styles * fix(AspectRatio): remove unsupported float: inline-start * fix(ContainedList): redefine tag tokens in contained list * fix(ContainedList): keep story the same
* fix(components): fix RTL issues with components * style(components): more fixes for RTL styles * style(components): more fixes for RTL styles * fix(Popover): fix popover styles in RTL mode * fix(TreeView): fix RTL issues with TreeView * fix(Breadcrumb): fix small issue with overflow menu variant * fix(Popover): fix autoalign story issues
* fix(menuButton): fix RTL issues with MenuButton * fix(Menu): adjust caret rotation * refactor(menu): remove rtl override * fix(Menu): adjust story positioning, add CaretLeft for RTL mode * refactor(Menu): useLayoutContext to check if parent has direction set
a14c39b to
414e440
Compare
4ff158a to
e07c0db
Compare
Closes #13619
Converts all applicable layout properties to CSS Logical Properties . This will greatly enhance RTL support throughout the design system.
Changelog
New
stylelint-use-logicaladded tostylelint-config-carbonChanged
Testing / Reviewing
Ensure there are no regressions in LTR (default) mode. Use the Layout switcher to test each component in RTL mode and ensure it renders as expected