Skip to content

Improve support for nested media queries #118

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

Merged
merged 1 commit into from
Jul 15, 2021
Merged

Conversation

lephe
Copy link
Contributor

@lephe lephe commented Jul 14, 2021

This PR changes the way Block.parse() handles media queries in order to operate more locally and recursively. This makes arbitrary nested media queries possible, which fixes #61, #65, and allows some constructions known not to work before.

Since this is quite a technical change, I assume some tweaking could be required to better integrate it in the code base. I'm submitting this in a state where bug test cases are fixed and tests pass so that the code can be reviewed. ^^

The main change in paradigm is to operate locally, meaning that every block handles themselves plus their direct children only. This is made possible by returning a list of nodes when some media queries need to be separated. The prototypical examples are as follow:

  • .foo { .bar{...} @media x {...} } becomes [.foo .bar {...}, @media x { .foo{...} }]
  • @media x { .bar{...} @media y {...} } becomes [@media x { .bar{...} }, @media x and y {...}]

By doing this at every level recursively, media queries naturally split their parents and bubble up from any nested structure.

In terms of code, media queries within the current node are sorted into inner_media_queries during the initial traversal of children, then inspected individually. New media queries alongside the current node are stored in sibling_media_queries and returned at the end.

Because the structure changes along the way, new blocks are created based off the .parsed and .inner of children rather than their .tokens[], which is unsuitable because it has media queries in an incorrect structure.

In terms of tests:

Here are the changes I noted needed some input:

  • Block.parse() now returns a list; if places in the code other than the parser and Block.parse() itself use it, adjustments may be needed.
  • The code previously re-used a block_name attribute for some blocks. I didn't find the purpose of the attribute from the few occurrences in the code, so I don't know when to check for it.
  • When a node with media queries is found, the code previously removed it from scope. When operating recursively the current node is often nowhere in scope (at index -2 or elsewhere). I didn't manage to understand exactly when this is the case, so I don't know precisely when to remove. I assume some variable visibility questions are related.
  • Because children are split in three (self.parsed, self.inner and inner_media_queries), the order of children tends to change during compilation. I don't think this is a problem as these three groups cannot have the same priority for properties, but I guess I should mention it.

I hope this helps. ^^

Verified

This commit was signed with the committer’s verified signature.
lephe Sébastien Michelland
Fixes lesscpy#61 and lesscpy#65 (plus some constructions known not to work before).
@lephe
Copy link
Contributor Author

lephe commented Jul 14, 2021

I realized I left the re-parsing case (self.parsed = True) in a testing state, this is now cleaned (it makes the diff smaller too).

@saschpe saschpe self-assigned this Jul 15, 2021
Copy link
Contributor

@saschpe saschpe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@saschpe saschpe merged commit 250d327 into lesscpy:master Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nested @media queries sometimes loose outer scope
2 participants