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

@starting-style doesn't nest correctly #4267

Open
mariusGundersen opened this issue Apr 10, 2024 · 13 comments
Open

@starting-style doesn't nest correctly #4267

mariusGundersen opened this issue Apr 10, 2024 · 13 comments

Comments

@mariusGundersen
Copy link

To reproduce:

Playground link

[popover]:popover-open {
  opacity: 1;
  transform: scaleX(1);

  @starting-style {
    opacity: 0;
    transform: scaleX(0);
  }
}

Current behavior:

This produces the following output, which is clearly incorrect:

[popover]:popover-open {
  opacity: 1;
  transform: scaleX(1);
}
@starting-style {
  opacity: 0;
  transform: scaleX(0);
}

Expected behavior:

It should produce the following css:

[popover]:popover-open {
  opacity: 1;
  transform: scaleX(1);

  @starting-style {
    opacity: 0;
    transform: scaleX(0);
  }
}

Environment information:

  • less version: 4.2.0

This is based on the sample code from mdn: https://developer.mozilla.org/en-US/docs/Web/CSS/@starting-style

@less less deleted a comment from dosubot bot Apr 10, 2024
@matthew-dean
Copy link
Member

@mariusGundersen Experimental CSS, by definition, is generally not supported by Less. That said, Less's default behavior of bubbling any at-rule that is undefined should probably be re-thought, or an escape-hatch provided. So this is more a feature request than a bug.

@rejhgadellaa
Copy link

With more and more @-rules being added (e.g. @starting-style and @position-try) and browsers supporting nested css (and thus @-rules), I think the default behavior of Less should indeed be changed. Should we move this into a separate ticket?

@rejhgadellaa
Copy link

Additional information:

@starting-style is now supported on Chromium and WebKit. Firefox has it implemented and plans to enable it soon.

@shrpne
Copy link

shrpne commented Aug 14, 2024

@starting-style is supported in Firefox now from 129 version

@mariusGundersen
Copy link
Author

Experimental CSS, by definition, is generally not supported by Less.

Starting style is not experimental anymore

@ogdakke
Copy link

ogdakke commented Oct 8, 2024

Yup, this should be supported now. Any timeline on this?

@puckowski
Copy link
Contributor

I have working code for @starting-style support on top of Less 4.2.1 (4.2.1 is yet to be published to npm) on my machine. I'll get the support into my fork https://github.com/puckowski/less.js. The fork also has support for the @scope at-rule. master branch will be clear when @starting-style support is ready.

If things go well I'll prepare a PR to this repo, but no guarantee of code merge or that a release will be published.

I maintain my fork since I use it for some of my projects, so PRs are welcome there.

@puckowski
Copy link
Contributor

Less.js 4.2.1 fork release with support for CSS Container Queries, Media Queries Level 4, starting-style at-rule, and scope at-rule support with all tests passing: https://github.com/puckowski/less.js/releases/tag/4.2.1-container-mq4-tr2-scope-1

I'll likely get around to submitting a PR to this repo just for @starting-style next weekend.

If you use the fork and run into an issue, feel free to open an issue on the fork.

@matthew-dean
Copy link
Member

@puckowski v4.2.1 released - https://www.npmjs.com/package/less - it may have some pulls your fork does not.

Btw, this repo could definitely use more core contributors.

@puckowski
Copy link
Contributor

@matthew-dean

Thank you for publishing Less.js 4.2.1.

I do not have access otherwise I would help, but release 4.2.1 doesn't look published here on GitHub Releases, just FYI. I know some users have asked about that in the past.

@puckowski
Copy link
Contributor

PR #4289 should resolve this issue. I'll support any requested revisions until it gets merged.

@jpattishall-ebay
Copy link

Hi @puckowski, out of curiosity, will this also address @container in @media not properly nesting?

Currently I see in 4.2.1 Playground I see this:

@media only screen and (min-width: 768px) {
  @container (min-width: 500px) {
    .primary-content {
      font-size: 1rem;
    }
  }
}

results in:

@container only screen and (min-width: 768px) and (min-width: 500px) {
  .primary-content {
    font-size: 1rem;
  }
}

But I see 4.1.3 produced the expected output:

@media only screen and (min-width: 768px) {
  @container (min-width: 500px) {
    .primary-content {
      font-size: 1rem;
    }
  }
}

Thanks!

@puckowski
Copy link
Contributor

@jpattishall-ebay PR #4289 does not resolve/address the issue you are describing. Thank you for your comment and for raising that separate issue. I was not previously aware of that issue.

I've taken a look at your issue on Less.js 4.2.1 and I believe I have a fix, but I need some time for further testing and revision. I'll try to prepare a separate PR by Friday 12/20/24.

I'll likely get my fix into my fork and cut a release before I file the PR, so I will notify you if you'd like to use that while waiting on the official Less.js release.

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

No branches or pull requests

7 participants