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.

Sign up for GitHub

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

Jump to bottom

when rtl is activated, ltr styles apply too #263

Open
sassanh opened this issue Oct 7, 2024 · 2 comments
Open

when rtl is activated, ltr styles apply too #263

sassanh opened this issue Oct 7, 2024 · 2 comments
Assignees
@brijeshb42
Labels
package: system Specific to @mui/system status: waiting for maintainer These issues haven't been looked at yet by a maintainer

Comments

@sassanh
Copy link

sassanh commented Oct 7, 2024 •
edited by oliviertassinari
Loading

Steps to reproduce

Kindly read this issue for the reproduction steps and as initial context of this issue.

Now I set generateForBothDir to true here

When I add dir="rtl" to the <html> tag, the rtl version of styles apply, but at the same time the ltr version of styles apply too, because as far as I understand nothing is preventing them. Should we change

pigment-css/packages/pigment-css-react/src/utils/preprocessor.ts

Line 59 in f1a32ff

if (generateForBothDir) {

to something like this:

  if (generateForBothDir) {
    css += stylis(
      `${getDirSelector(defaultDirection === "ltr" ? "ltr" : "rtl")} ${!isGlobal ? `${selector}{${cssText}}` : cssText}`,
    );
    css += stylis(
      `${getDirSelector(defaultDirection === "ltr" ? "rtl" : "ltr")} ${!isGlobal ? `${selector}{${cssText}}` : cssText}`,
      serializerRtl
    );
  }
  else {
    css += stylis(!isGlobal ? `${selector}{${cssText}}` : cssText);
  }

and add to the docs that when using generatedForBothDir one should always set the dir attribute of the html tag.

If it sounds rational to you, I can create a pull request.

Your environment

pigment 0.0.24
Safari 18.1

Search keywords: rtl
@sassanh sassanh added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 7, 2024
@zannager zannager added the package: system Specific to @mui/system label Oct 7, 2024
@brijeshb42
Copy link
Contributor

Can you try with generateForBothDir set to false ?

@sassanh
Copy link
Author

sassanh commented Oct 18, 2024

@brijeshb42
If I set generateForBothDir only one version of stylesheets will be generated and this issue would become a non-issue. Or maybe I'm misunderstood about the bi-directional API, is it possible to have my website bidirectional with generateForBothDir set to false?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@brijeshb42 brijeshb42

Labels
package: system Specific to @mui/system status: waiting for maintainer These issues haven't been looked at yet by a maintainer
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@brijeshb42 @sassanh @zannager