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

Property name interpolation #1600

Merged
merged 2 commits into from
Dec 19, 2013
Merged

Property name interpolation #1600

merged 2 commits into from
Dec 19, 2013

Conversation

seven-phases-max
Copy link
Member

Support for "property name interpolation" feature (#36, #698).

Just a few comments on implementation: Actually I'm not too happy by the fact that I did not manage to reuse existing parse functions ($() and variableCurly() in particular). In fact I started to write all this with those functions in mind but then realized that most likely it will end with quite cluttered and iffy code because of whitespace backtracking. So I simply switched to this "self-contained" parsing method.

@lukeapage
Copy link
Member

This approach works, but I have always meant to improve backtracking to support multiple saves as it constantly comes up when fixing bugs. I won't stop this getting merged because of that though.

@jonschlinkert
Copy link
Contributor

👍

@Soviut
Copy link

Soviut commented Nov 5, 2013

+1 here too

@cwygoda
Copy link

cwygoda commented Nov 21, 2013

👍

merge = true;
name = name.substr(0, name.length - 1);
}
merge = name.pop && (name.pop() === "+");
Copy link
Member

Choose a reason for hiding this comment

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

does this work in chrome? won't this remove the last character if it isn't + ?

Copy link
Member Author

Choose a reason for hiding this comment

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

won't this remove the last character if it isn't + ?

Hmm, now when you mentioned it I start to concern myself. With that code I assumed that pop is an Array only function and it is not available for strings. I.e. the first name.pop is just isArray? and the second name.pop removes the last array item which is always "+" or "" if the name is an array.
But now I see how confusing it may look so let me know if you'd prefer me to update this part with a less cryptic condition (using Array.isArray or so).

Or maybe it would be even better to add a comment there like

the returned property name is always an array of form ["string-1", ..., "string-n", ""] or ["string-1", ..., "string-n", "+"]

or a sort of?.

@seven-phases-max
Copy link
Member Author

temporary closed since there's no way to merge it after perf. optimization update. I'll try to update it soon.

@seven-phases-max
Copy link
Member Author

Updated with "perf. opt." changes and reopened.


// a name returned by this.ruleProperty() is always an array of the form:
// ["", "string-1", ..., "string-n", ""] or ["", "string-1", ..., "string-n", "+"]
merge = name.pop && (name.pop() === "+");
Copy link
Member Author

Choose a reason for hiding this comment

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

I hope the comment helps with these name.pops.

Copy link
Member

Choose a reason for hiding this comment

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

I'm still against this but I will merge in next, in the face of popular
support.
I just wish we had a tighter mire restrictive syntax to avoid future
clashes but I'm at a loss to come up with a better one (that also
distinguishes from selectors.

Copy link
Member Author

Choose a reason for hiding this comment

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

Btw, I impemented it in the face of popular support too of course, but personally for me the benefit of this feature is not any kind of autoprefixing (which I don't care of at all) but an ability to fully automate whatever CSS {property: value} generation. Imagine this (somewhere in the future): .make-animation(@property, @base-value, @animation-template-function) - where @property is any property (or even a list of properties), @base-value is some base value (or a list) for that property(s) and @animation-template-function is some mixin template that defines actual animation parameters (e.g. curve, time, number of frames and actual value(s) changes for each frame etc.). This will be impossible w/o "property name interpolation". E.g. something like (in pseudo-syntax):

#z {
    .make-animation(color, red, <rainbow, 20 frames, 10 sec, accelerated-circe>);
    .make-animation(width, 20px, <jumping, 20 frames, 10 sec, accelerated-circe-inverted>);
    &:hover {.make-animation(background-color, blue, <fade-in, 10 frames, 2 sec, ease-in>);}
} // -> 

For the moment something like above will require ~50 hand-written @keyframe items + ~20 lines of supporting parameters + tons of parens and brackets, and this all hardcoded for that 3 specific CSS properties...
This is just (a bit crazy) one of possible use-cases of course, but I believe we'll invent more interesting automating stuff when this goes in. Just yet another level of abstraction.

Copy link
Member

Choose a reason for hiding this comment

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

interesting use-case.

@lukeapage lukeapage merged commit ebdadae into less:master Dec 19, 2013
@lukeapage
Copy link
Member

merged!

@seven-phases-max seven-phases-max deleted the property-name-interp branch December 19, 2013 08:50
@seven-phases-max
Copy link
Member Author

Thanks!

@fahad19
Copy link

fahad19 commented Dec 24, 2013

💯

@seven-phases-max seven-phases-max mentioned this pull request Jan 9, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants