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

postcss-logical, v8 breaks logical properties semantic #832

Closed
3 tasks done
felixmosh opened this issue Feb 2, 2023 · 4 comments
Closed
3 tasks done

postcss-logical, v8 breaks logical properties semantic #832

felixmosh opened this issue Feb 2, 2023 · 4 comments
Labels

Comments

@felixmosh
Copy link

What would you want to propose?

In previous version (v7) the css that was generated supports both direction (like it should be with logical properties).

// this
.test {
  margin-inline-start: 1rem;
}

// v7
[dir="ltr"] .test {
  margin-left: 1rem;
}
[dir="rtl"] .test {
  margin-right: 1rem;
}

This preserves the same semantics of logical properties, the "same" css code supports both directions.

Starting v8, it generates only one direction (as specified in the plugin options), this breaks the semantics above.

Suggested solution

Maybe it is possible to add an plugin option to generate both direction at once, like it was in v7

Additional context

No response

Validations

  • Follow our Code of Conduct
  • Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

Would you like to open a PR for this feature?

  • I'm willing to open a PR
@felixmosh felixmosh added the feature request New feature or request label Feb 2, 2023
@felixmosh felixmosh changed the title postcss-logical, add fallback support postcss-logical, v8 breaks logical properties semantic Feb 2, 2023
@Antonio-Laguna
Copy link
Member

@felixmosh thanks for reaching out!

The reason we got rid of that is because inline could also be on the vertical axis. On top of that, that selector you're adding it's also more specific than the original (which is wrong). It's 0,1,0 vs 0,2,0

@felixmosh
Copy link
Author

But the semantics is wrong, so how does it solves this?
You require users to compile their css twice, and make load the correct file... it is much worse in my opinion then the specificity bump.

Regarding vertical axis, I'm not sure what is the problem with previous implementation...

@Antonio-Laguna
Copy link
Member

I don't think the semantics is wrong.

Compiling CSS Twice is a sad side-effect of this change but it's rare that you need this option so we thought it to be an acceptable tradeoff.

If you were doing a site in Chinese:

margin-inline: 1px;

Should become

margin-top: 1px;
margin-bottom: 1px;

Because inline direction is top to bottom in that language which can't just be covered with [dir]

@felixmosh
Copy link
Author

So compiling twice the css is the recommended way to go? it won't work in meta frameworks like Next.js, Remix since you don't own the build.
Sad, I'll stick to v7, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants