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

Variable selector "string" improperly parses comma #1421

Closed
scottgit opened this issue Jul 15, 2013 · 9 comments
Closed

Variable selector "string" improperly parses comma #1421

scottgit opened this issue Jul 15, 2013 · 9 comments

Comments

@scottgit
Copy link

This StackOverflow question lead me to test what I thought would work, but failed. Namely, this example:

@inputs: input[type=text], input[type=email], input[type=password], textarea;

@{inputs} {
  &:focus {/*properties*/}
} 

Did not produce what was expected, as it failed to recognize the commas as separators of individual selectors. So instead of this expected output:

input[type=text]:focus,
input[type=email]:focus,
input[type=password]:focus,
textarea:focus {
  /*properties*/
}

It produced this unexpected output of just adding the :focus on the end of the whole variable value:

input[type=text], input[type=email], input[type=password], textarea:focus {
  /*properties*/
}

If the variable interpretation for selectors uses the same algorithm as when a variable is in a string, I can see "why" it is doing it that way, but it would seem to me that, at least for the case of the , in CSS, the variable interpretation needs to recognize it is dealing with a selector at that point, and should then take the next step and parse each of the commas to separate selectors, so that all nested uses of & will attach to all the appropriate selector strings.

@lukeapage
Copy link
Member

There are several issues raised all around the fact less does not parse
variable selectors.. part of this is because a user may have escaped the
selector because less couldn't parse it.. however, generally it seems there
is a lot of support to at least try to re-parse once the variable is
evaluated.

@pavlovich
Copy link

I would like to see this issue addressed as well. It would be nice to be able to define a set of selectors and refer to them in various places throughout my LESS files via a variable name.

@scottgit
Copy link
Author

@pavlovich While I would still like to see the capabilities noted in my original issue, there is a work around to get the functionality of assigning properties to a set of selectors defined in a variable using the capabilities of LESS 1.7. Consider this:

@inputs: {input[type=text], input[type=email], input[type=password], textarea {.getProps()}};
@pseudo: ~':focus';
@props: { /*properties */};

.setProps(@selectors; @props; @extension: ~'') {
  @selectors();
  .getProps() {
     &@{extension} { @props(); }
  }
} 

.setProps(@inputs; @props; @pseudo);

Which outputs:

input[type=text]:focus,
input[type=email]:focus,
input[type=password]:focus,
textarea:focus {
  /*properties */
}

So I can assign a pseudo class if I want or not, and pass a set of properties as needed. The key is to put the mixin .getProps() call into the block that is defining the selectors for variable @inputs, which is then called as a local mixin inside .setProps() so that the properties get put where we want them. The @extension could be any escaped string to add to each selector as an additional part of the selector string. Above I do :focus as a pseudo class, but I could have done ~' > div + div' as a child and sibling combination, or whatever.

@erquhart
Copy link

@seven-phases-max so ReadyForImplementation means this is coming, but Low Priority means don't hold your breath. Any idea if this will happen in the near future?

@seven-phases-max
Copy link
Member

ReadyForImplementation usually only means it's just decided that it may be in if someone implements it (not necessary coming soon or ever at all if nobody finds time to implement it).

@calvinjuarez
Copy link
Member

Something to think about:
http://dev.w3.org/csswg/css-extensions/#custom-selector

@matthew-dean
Copy link
Member

I think all that's needed here is for Less to treat comma-separated lists equally. A variable that holds a list should be expanded with & in the same way as a list of selectors. In other words, we don't actually need to know that the list contains valid selectors in order for this to work, AFAIK.

@matthew-dean
Copy link
Member

Elevating priority because of #2964.

@matthew-dean
Copy link
Member

matthew-dean commented Jun 2, 2018

I have a work-in-progress here: https://github.com/matthew-dean/less.js/tree/bugfix-1241 but the parsing is way more complicated than I thought when I started.

Basically, the normal evaluation pattern of Less is stuff gets evaluated and then returns an evaluated copy of itself. In this case, the variable reference is added as an Element in a Selector in an array of selectors inside Ruleset. This can be re-parsed at eval-time as selectors, but then you have to correctly re-organize the tree so that what was thought to be an element is actually an entire list of selectors, so you need to merge those selectors into the top-level selectors BUT they may NOT be selectors (and may be elements), so they need to be preserved and just evaluated.

If someone wants to take the work on that branch and run with it, be my guest. I think that's all the time I'll have for it.

Never mind, got it to work! ^_^

matthew-dean added a commit that referenced this issue Jun 16, 2018
…3217)

* Adds passing test from #3098
* Added passing test example from #1817
* Allow lists to be re-evaluated as selectors (Fixes #1694)
matthew-dean added a commit that referenced this issue Jun 17, 2018
matthew-dean added a commit that referenced this issue Jun 17, 2018
Revert "Fixes #1421 - re-parses variable-interpolated elements to selectors"
matthew-dean added a commit that referenced this issue Jun 25, 2018
…no.2) (#3227)

* Fix element to selector list conversion, passing all tests!
* Add passing test from #3098
* Added passing test example from #1817
* Allow lists to be re-evaluated as selectors (Fixes #1694)
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