less / less.js

Less. The dynamic stylesheet language.
http://lesscss.org
Apache License 2.0
17.01k stars 3.41k forks source link

Support for recently added CSS functionality container-queries #3766

Closed sneridagh closed 1 year ago

sneridagh commented 1 year ago

As pointed out by @treponat, it seems support for @container queries should be implemented in less.

By tomorrow (12/12/2022) Firefox will release 108 with support for container queries, completing all the major browsers support for the feature.

Discussed in https://github.com/less/less.js/discussions/3759

Originally posted by **treponat** November 3, 2022 Recently some of the browser started to support a new feature that would allow to create responsive design based on the parent element rather than client's viewport. https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Container_Queries It would be wonderful if LESS would also start to fully support this feature. Currently you can use container functionality in .less files, but the hierarchical structure of css blocks is not working as expected for the container syntax. for example ``` .widget.discoverresults, .widget.repositoriesresults { container-type: inline-size; @container (max-width: 350px) { .cite { .wdr-authors { display: none; } } } } ``` would produce this css blocks ``` .widget.discoverresults, .widget.repositoriesresults { container-type: inline-size; } @container (max-width: 350px) { .cite .wdr-authors { display: none; } } ``` instead of ``` .widget.discoverresults, .widget.repositoriesresults { container-type: inline-size; } @container (max-width: 350px) { .widget.discoverresults, .widget.repositoriesresults { .cite .wdr-authors { display: none; } } } ```
iChenLei commented 1 year ago

cc @matthew-dean

levito commented 1 year ago

I found a workaround with saving the selector to a variable using the plugin https://github.com/seven-phases-max/less-plugin-reflections as suggested here: https://github.com/less/less.js/issues/3053#issuecomment-299707813

.container(@from, @rules) {
  .container(@from, @to: 0, @rules);
}
.container(@from, @to, @rules) {
  @selector: current-selector();

  .return(@selector) when (@from = 0) {
    @container (max-width: @to) {
      @{selector} {
        @rules();
      }
    }
  }
  .return(@selector) when (@to = 0) {
    @container (min-width: @from) {
      @{selector} {
        @rules();
      }
    }
  }
  .return(@selector) when (default()) {
    @container (min-width: @from) and (max-width: @to) {
      @{selector} {
        @rules();
      }
    }
  }
  .return(@selector);
}

Usage:

.deeply .nested .selector {
  .container(300px, {
    content: "container min width of 300px";
  }
  .container(0, 700px, {
    .going .deeper {
      content: "container max width of 700px";
    }
  }
}
matthew-dean commented 1 year ago

Oh I see, you're expecting @container to act like @media or @supports? That's reasonable.

sneridagh commented 1 year ago

@matthew-dean yeah, that's the point :) BTW, I forgot to mention that SCSS has already support for this behavior.

rejhgadellaa commented 1 year ago

Just ran into this issue. Is there any timeframe on how long this could take to land this in a release? I'm trying to decide if I should wait a few weeks or do everything the old fashioned way for now and then refactor in a couple of months or so?

abhishek-kumar-91 commented 1 year ago

I want to work on this issue, please assign me.

matthew-dean commented 1 year ago

@abhishek-kumar-91 Go for it!

matthew-dean commented 1 year ago

@abhishek-kumar-91

I presume that what you'll want to do is extend from (or refactor) the Media tree type. Then you'll probably want to alter the media parser to generalize it for @container queries.

This is called within the atrule parser early, for @import, @plugin, and @media, as they have special handling rules, so I presume the line will end up looking like:

value = this['import']() || this.plugin() || this.mediaOrContainer();

And then, of course, writing css / less tests to verify input / output. Make sense?

levito commented 1 year ago

How are things going with this? When working on this, it would be awesome to also support container style queries, which I think should be pretty straightforward since it's syntactically very similar to named container queries.

tatof commented 1 year ago

Any news on the @container css media queries?

puckowski commented 1 year ago

I have a fork of Less.js 4.1.3 that supports container queries. Passes all tests, plus additional tests for container queries.

This:

.widget.discoverresults, .widget.repositoriesresults {
        container-type: inline-size;

        @container (max-width: 350px) {           
            .cite {
                .wdr-authors {
                    display: none;
                }
            }
        }
}

becomes this:

.widget.discoverresults, 
.widget.repositoriesresults {
    container-type: inline-size;
}

@container (max-width: 350px) {           
    .widget.discoverresults .cite .wdr-authors,
    .widget.repositoriesresults .cite .wdr-authors {
        display: none;
    }
}

I plan on making the fork public sometime later today (07/19/23) and creating a pull request sometime within the next couple of days.

Size of the minified bundle goes up by around 2KB.

levito commented 1 year ago

Awesome, @puckowski, this is really great news!

tatof commented 1 year ago

yeah great @puckowski 👍

matthew-dean commented 1 year ago

Nice!

puckowski commented 1 year ago

I've used Less.js before, but I am new to the codebase as of yesterday.

I added support for container queries and tests report: All Passed 215 run Test Less here: https://github.com/puckowski/less.js/blob/3c26737190b8690c1908bb05fad7eb55ff14806f/packages/test-data/less/_main/container.less

Tomorrow I plan on reviewing the container query specification and adding more tests.

I also need to polish my forked pull request, with an eye for:

Ran out of time today, but I hope to get a pull request to the Less.js official repository within the next couple of days.

You can track the progress here: https://github.com/puckowski/less.js/pull/1

puckowski commented 1 year ago

Will review everything one last time tomorrow (07/21/23) and then submit a pull request to this repository.

I created a build of Less.js 4.1.3 with support for CSS Container Queries here: https://github.com/puckowski/less.js/releases/tag/4.1.3-container-queries-1

If you test the above build and find an issue, let me know.

I think I added enough tests here: https://github.com/puckowski/less.js/pull/1/files#diff-9880faf95a44167ced4666a6eb1c9259f577483ce79944aa8b356e033a9cb239

https://github.com/puckowski/less.js/pull/1

Performance:

I have found performance is roughly the same, fluctuating slightly between runs of the benchmark based on whatever the host is doing.

There are a couple of quick checks added for at rules, and if the checks pass a small amount of additional code is run relative to the default branch of execution. In all, performance should be level or only slightly slower when evaluating many at rules.

Pull request branch:

----------------------
Parsing
----------------------
Min. Time: 6 ms
Max. Time: 11 ms
Total Average Time: 8 ms (12070 KB/s)
+/- 67%

----------------------
Evaluation
----------------------
Min. Time: 16 ms
Max. Time: 36 ms
Total Average Time: 19 ms (5038 KB/s)
+/- 106%

----------------------
Render Time
----------------------
Min. Time: 23 ms
Max. Time: 44 ms
Total Average Time: 27 ms (3554 KB/s)
+/- 79%

master branch:

----------------------
Parsing
----------------------
Min. Time: 7 ms
Max. Time: 9 ms
Total Average Time: 8 ms (12309 KB/s)
+/- 33%

----------------------
Evaluation
----------------------
Min. Time: 16 ms
Max. Time: 36 ms
Total Average Time: 19 ms (5026 KB/s)
+/- 102%

----------------------
Render Time
----------------------
Min. Time: 23 ms
Max. Time: 43 ms
Total Average Time: 27 ms (3569 KB/s)
+/- 72%

Tests:

I evaluated the CSS Container Query specification add added appropriate tests, which are passing and have been manually reviewed. All the original tests pass.

- Test Sync _main/import: OK

All Passed 213 run
- Test Sync _main/plugin: OK

All Passed 214 run
- Test Sync math/strict/css: OK

All Passed 215 run

Bundle size:

Less.js 4.1.3 Official minified: 146,335 bytes Less.js 4.1.3 Pull Request minified: 148,260 bytes Delta: 1,925 bytes

Behavior of media at-rule with invalid query in parens:

Query in parens syntax comes from CSS Container Query specification, but I do not believe media at rules support them. I searched online and did some tests in a CodePen.

Both master and the pull request branch result in the following when media at rule tries to use query in parens:

ParseError: Missing closing ')'

Extensibility:

The pull request lays the groundwork for adding scope at rules with minimal additional effort.

puckowski commented 1 year ago

https://github.com/less/less.js/pull/3811 adds support for CSS Container Queries to Less.js.

I updated the build here: https://github.com/puckowski/less.js/releases/tag/4.1.3-container-queries-2 (there was an issue with the first release in my fork). If you download, test, and find any issues, please let me know.

Will refactor pull 3811 as needed until it gets merged.