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-float-and-clear not including [dir="rtl"] version of ruleset #958

Closed
2 of 3 tasks
mherchel opened this issue May 1, 2023 · 6 comments
Closed
2 of 3 tasks

Comments

@mherchel
Copy link

mherchel commented May 1, 2023

Bug description

I'm not sure if this is a bug or a feature (because the documentation doesn't say that this should be compiled this way)

The issue is that PostCSS Logical will used to compile CSS and include RTL versions (assuming default is LTR).

Example source:

.test { padding-inline-start: 10px; }

Example output:

.test { padding-left: 10px; }
[dir="rtl"] .test { padding-right: 10px; }

However, I can't see any option within PostCSS Logical Float And Clear to do the same.

Source CSS

.test { float: inline-start; }

Expected CSS

.test { float: left; }
[dir="rtl"] .test { float: right; }

Actual CSS

.test { float: left; }

Does it happen with npx @csstools/csstools-cli <plugin-name> minimal-example.css?

N/A

Debug output

No response

Extra config

No response

What plugin are you experiencing this issue on?

PostCSS Preset Env, PostCSS Logical Float and Clear

Plugin version

1.0.1

What OS are you experiencing this on?

macOS

Node Version

16.13.1

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 bug?

  • I'm willing to open a PR
@mherchel
Copy link
Author

mherchel commented May 1, 2023

Note that it looks like PostCSS Logical doesn't do a [dir="rtl"] prefix anymore. It appears that this change appeared in #740, but the link to the Changelog is broken

@romainmenke
Copy link
Member

romainmenke commented May 1, 2023

Hi @mherchel,
Thank you for reaching out!

I've added updated changelog links where needed : https://github.com/csstools/postcss-plugins/blob/main/plugins/postcss-logical/CHANGELOG.md#600-january-24-2023

This change was very breaking and although it was announced, it still surprised many people :)
We've created a wiki entry to explain all the issues with the old approach (adding :dir(rtl)) in detail : https://github.com/csstools/postcss-plugins/wiki/PostCSS-Logical-Issues

@mherchel
Copy link
Author

mherchel commented May 1, 2023

yeah, I totally get the issues from the old approach (especially with the specificity). Is there a way to revert to that behavior though? We're using PostCSS Preset ENV in Drupal core, and we don't have any way to predict if a site's language is going to be LTR or RTL, hence the need for that functionality.

@romainmenke
Copy link
Member

Is there a way to revert to that behavior though?

No.

But there is one alternative we are considering.
see : #892 (comment)

You write selectors with [dir] and use logical props/values inside those rules.
Then the plugin could generate fallbacks with [dir=ltr] and [dir=rtl].

The specificity would remain the same, so it doesn't have that specific issue.

But this approach still suffers from bloated outputs.
Because one rule generates two new rules you easily get a lot of bloat.


Is it possible to generate multiple output stylesheets for Drupal core?
One for ltr, one for rtl, ...

And then leave it up to theme builders / users to include the correct one depending on settings, ...

@romainmenke
Copy link
Member

Also a valid approach is to fork the old logical plugin and keep using it directly (not through PostCSS Preset Env).

This allows you to keep using dir while also having access to the latest features from PostCSS Preset Env.

The source code can be found on the postcss-preset-env--v7 branch :
https://github.com/csstools/postcss-plugins/tree/postcss-preset-env--v7/plugins/postcss-logical

If you do decide to fork from that point we are happy to assist in any way.

@mherchel
Copy link
Author

mherchel commented May 1, 2023

Thanks for the response. At this point all of our supported browsers support logical properties (yay!) except for float, clear, etc, and tbh, there's not too much more of that. My thought is we'll just keep as-is until browser vendors include this.

Thanks again for all of the excellent work here :D

@mherchel mherchel closed this as completed May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants