postcss-plugins icon indicating copy to clipboard operation
postcss-plugins copied to clipboard

postcss-logical : use variables

Open romainmenke opened this issue 4 years ago • 14 comments

related issues :

  • https://github.com/csstools/postcss-plugins/issues/90
  • https://github.com/csstools/postcss-logical/issues/26
  • https://github.com/csstools/postcss-logical/issues/37

The current implementation of postcss-logical modifies the selector by adding :dir(lrt|rtl).

This changes specificity and doesn't work for @keyframes. It also breaks down in code that uses multiple dir values in a single document

<html dir="ltr">
 <body>
  <div dir"rtl">
    <p>example of "rtl" text</p>
  </div>
 </body>
</html>

I think that using CSS Variables would resolve most of these issues. The tradeoff is that we can't have both ltr and rtl in a single document in older browsers that do not support CSS Variables.

https://caniuse.com/css-variables

Browsers that do not support variables would fallback to ltr.

The variable name used in the example below is a shortened md5 hash of the current "tree", combined with a simple incrementing counter. This gives a unique variable name that is still consistent between multiple builds from the same source.


This change does not effect CSS output when the dir plugin option is provided.


Input CSS :

test-float-3 {
	float: inline-end;
}

@media screen and (min-width: 640px) {
	test-float-3 {
		float: center;
	}
}

Before this change :

test-float-3:dir(ltr) {
	float: right;
}

test-float-3:dir(rtl) {
	float: left;
}

@media screen and (min-width: 640px) {
	test-float-3 {
		float: center;
	}
}

After this change :

:dir(ltr) {
	--logical-b352c2-2: right;
}

:dir(rtl) {
	--logical-b352c2-2: left;
}

test-float-3 {
	float: right;
	float: var(--logical-b352c2-2);
}

@media screen and (min-width: 640px) {
	test-float-3 {
		float: center;
	}
}

Any thoughts on this?

romainmenke avatar Dec 24 '21 18:12 romainmenke

I tried to implement this strategy in @parcel/css as well - https://github.com/parcel-bundler/parcel-css/pull/30. I ran into one problem though, which is that it doesn't really work if the value itself also contains variables. It only works if the variables are defined at or above the element with the dir attribute, not if the variable is defined somewhere within.

some-element {
  --radius: 4px;
  border-start-start-radius: var(--radius);
}

produces

[dir="ltr"] {
  --top-left-radius: var(--radius);
  --top-right-radius: ;
}

[dir="rtl"] {
  --top-left-radius: ;
  --top-right-radius: var(--radius);
}

some-element {
  --radius: 4px;
  border-top-left-radius: var(--top-left-radius);
  border-top-right-radius: var(--top-right-radius);
}

That doesn't work unless some-element itself has a dir attribute.

Unfortunately, I haven't found a workaround yet... 😞

devongovett avatar Jan 03 '22 02:01 devongovett

Ok, found an alternative approach: https://github.com/parcel-bundler/parcel-css/pull/30#issuecomment-1003871441

devongovett avatar Jan 03 '22 05:01 devongovett

@devongovett Thank you for posting this here! Your alternative looks very promising and will try to work this out here as soon as possible :)

romainmenke avatar Jan 03 '22 07:01 romainmenke

FYI, discovered another problem with this approach.

.foo {
  border-radius: 20px;
}

.foo {
  border-start-start-radius: 0;
}

transforms to:

.foo {
  border-radius: 20px;
}

.foo {
  border-top-left-radius: var(--ltr, 0);
  border-top-right-radius: var(--rtl, 0);
}

[dir="ltr"] {
  --ltr: initial;
  --rtl: ;
}

[dir="rtl"] {
  --ltr: ;
  --rtl: initial;
}

The problem is that, for example in ltr, border-top-right-radius will resolve to " ". That's invalid for this property normally, but at the point variables are substituted, the cascaded value that would normally be a fallback during parsing has been thrown away. It'll just become the default value (0) instead. This is the invalid at computed value time part of the spec. AFAICT there is no way around this.

I'm thinking of changing Parcel CSS to use the :lang selector for this instead, with a list of languages that are normally written right-to-left. It's not 100% right, but I think it's the closest since it works with nested direction changes. Usually, when you use the dir attribute you also set the lang.

Using the dir attribute was also not quite right because direction changes can be made using the direction CSS property as well, and it can also be automatically inferred by the browser based on the content if no dir attribute is set. So there's no way to polyfill it 100% correctly, but I think lang is pretty close for most use cases. Curious if you have experience that suggests otherwise.

devongovett avatar Mar 31 '22 15:03 devongovett

@devongovett Thank you for this! I always appreciate it so much when you come back here and share your learnings!

Using the dir attribute was also not quite right because direction changes can be made using the direction CSS property as well, and it can also be automatically inferred by the browser based on the content if no dir attribute is set. So there's no way to polyfill it 100% correctly, but I think lang is pretty close for most use cases. Curious if you have experience that suggests otherwise.

This is where I gave up my last attempt. Reading through all the properties that can influence this and that this goes way beyond ltr and rtl.

I hope to pick this up again when I have time to take a deep dive.

I do wonder if maybe it's best to fallback everything to one base direction. If a true polyfill can't be achieved we can still save authors time. They won't have to manually provide fallback declarations for each logical property. The final result on the page will also be consistent and readable.

Maybe configurable which fallback is created?

romainmenke avatar Mar 31 '22 15:03 romainmenke

Yeah. My current idea is something like this:

.foo:not(:lang(ae, ar, arc, bcc, bqi, ckb, dv, fa, glk, he, ku, mzn, nqo, pnb, ps, sd, ug, ur, yi)) {
  /* ltr */
  border-top-left-radius: 0;
}

.foo:lang(ae, ar, arc, bcc, bqi, ckb, dv, fa, glk, he, ku, mzn, nqo, pnb, ps, sd, ug, ur, yi) {
  /* rtl */
  border-top-right-radius: 0;
}

This way, if no lang is set, the :not selector will apply and you'll get the LTR styles.

devongovett avatar Mar 31 '22 15:03 devongovett

That looks nice, and compression will make the overhead largely go away as it's always the same.

https://developer.mozilla.org/en-US/docs/Web/CSS/:lang#browser_compatibility

Didn't know that support for :lang was this good 🤔 Also makes sense to follow your approach on this. Makes it easier for users to switch tooling without painful migrations.

romainmenke avatar Mar 31 '22 15:03 romainmenke

Side note :

I should send a patch to fix the specificity display in VSCode. In my last PR I didn't account for :lang() and I don't think it works this way.

Screenshot 2022-03-31 at 17 26 27

done : https://github.com/microsoft/vscode-css-languageservice/pull/268

romainmenke avatar Mar 31 '22 15:03 romainmenke

yeah that seems weird. but it got me thinking. Pseudo elements including :lang and :dir raise the specificity of the selector by one element, so that means compiling the following wouldn't work correctly. 😢

.foo {
  border-radius: 20px;
}

.foo {
  border-start-start-radius: 0;
}

.foo {
  border-radius: 50px;
}

It could be possible to fix that using the :where selector to lower the specificity of the :lang to zero, but not sure it has enough browser support yet. Still, :lang might be the best we can do...

devongovett avatar Mar 31 '22 15:03 devongovett

We can raise the specificity of everything else by [0, 1, 0] with :not(.something-unlikely-to-exist).

It's messy but it works.

I initially started this PR to fix specificity issues with [dir="..."] :D

romainmenke avatar Mar 31 '22 15:03 romainmenke

Ha, nice hack!

devongovett avatar Mar 31 '22 15:03 devongovett

Hello, any updates here? I have the same problem with selectors specifity. Can i help with this PR somehow?

nnn3d avatar Oct 11 '22 17:10 nnn3d

Hi @nnn3d,

Unfortunately there isn't much you can do at this time for this issue. The only way to really fix this plugin is with breaking changes and that means that it will be part of postcss-preset-env version 8 and more specifically this batch of changes : https://github.com/csstools/postcss-plugins/issues/530

romainmenke avatar Oct 11 '22 17:10 romainmenke

Hi all, I have been following this PR closely, and think I have something working that avoids the issues related to the invalid at computed value time part of the spec. My solution does however require a JS polyfill that leverages the MutationObserver API to add is-{ltr,rtl} classes to every DOM element on the page. Would that be acceptable for this plugin? I see some other postcss plugins do require the use of JS as well - Example: https://github.com/csstools/postcss-plugins/tree/main/plugins/css-blank-pseudo.

SalimBensiali avatar Oct 16 '22 03:10 SalimBensiali

Hi @SalimBensiali Thank you for suggesting this! We however try to focus on CSS only transforms and fallbacks as these give developers a bit more flexibility. Only when absolutely needed do we create a JS polyfill. In this case we feel a CSS only solution will be ok.

romainmenke avatar Nov 07 '22 18:11 romainmenke

To everyone following along here, I am closing this pull request because I will not continue working on this issue within the same approach to solve it.

If you want to be notified of updates related to this issue it is best to subscribe to this issue : https://github.com/csstools/postcss-plugins/issues/90

romainmenke avatar Nov 07 '22 18:11 romainmenke