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

[Snackbar] Remove RTL direction specific logic #32808

Merged
merged 5 commits into from Jun 8, 2022

Conversation

aaarichter
Copy link
Contributor

@aaarichter aaarichter commented May 17, 2022

Background

The Mui page guidelines for RTL are to use the stylis-plugin-rtl (https://mui.com/material-ui/guides/right-to-left/) which handles rewriting directional css properties automatically. With RTL being handled within the component, stylis will assume LTR styles, even though they are already RTL. So it transforms the styles, which will result in LTR.

To avoid this issue, one has to remove RTL handling within the component.

Fixes #32794

@mui-bot
Copy link

mui-bot commented May 17, 2022

Details of bundle changes

Generated by 🚫 dangerJS against 9458b66

@mnajdova
Copy link
Member

I wanted to check this same thing when reviewing #31991 today. cc @ZeeshanTamboli

The changes makes sense 👍 Let's just make sure we don't break this again in the future by adding a screenshot test. You can follow what is done in https://github.com/mui/material-ui/blob/master/test/regressions/fixtures/Tooltip/PositionedTooltipsRtl.js and add a new one under /test/regressions/fixtures/Snackbar/.

@mnajdova mnajdova added component: snackbar This is the name of the generic UI component, not the React module! PR: needs test The pull request can't be merged labels May 18, 2022
@aaarichter
Copy link
Contributor Author

@mnajdova thanks for the link. I added those regression files

@ZeeshanTamboli
Copy link
Member

@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented May 20, 2022

I wanted to check this same thing when reviewing #31991 today. cc @ZeeshanTamboli

I will take a look. Will remove the rtl logic in multiple snackbars PR.

@ZeeshanTamboli ZeeshanTamboli changed the title fix(Snackbar): remove RTL specific logic. Fixes #32794 [Snackbar] Remove RTL direction specific logic May 20, 2022
@ZeeshanTamboli ZeeshanTamboli added bug 🐛 Something doesn't work and removed PR: needs test The pull request can't be merged labels May 20, 2022
ZeeshanTamboli added a commit to ZeeshanTamboli/material-ui that referenced this pull request May 20, 2022
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

I've just improved the regression tests, the rest looks great. Thanks for the contribution 👌

@mnajdova mnajdova merged commit 96f5a72 into mui:master Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: snackbar This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Snackbar] horizontal positioning is not changing in RTL
4 participants