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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support strings as property names #698

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
@sirlantis
Copy link
Contributor

sirlantis commented Mar 9, 2012

This is analogous to the recent 93b23d2 (support strings as selectors) commit.

Includes a regression test.

This fixes #36 and should make creating vendor-aware libraries/mixins much easier. An example where this could be useful:

.experimental(@property, @value) {
  ~"-webkit-@{property}": @value;
  ~"-moz-@{property}": @value;
  ~"-ms-@{property}": @value;
  ~"-o-@{property}": @value;
  ~"@{property}": @value;
}

.borderRadius(@value) {
  .experimental("border-radius", @value);
}

article {
  .borderRadius(5px);
}

// CSS OUTPUT

article {
  -webkit-border-radius: 5px;
  -moz-border-radius: 5px;
  -ms-border-radius: 5px;
  -o-border-radius: 5px;
  border-radius: 5px;
}

Also it should make @metaskills (see blogpost) happy. 馃巵

@stefanpearson

This comment has been minimized.

Copy link

stefanpearson commented Mar 12, 2012

This would be very useful to me. I want to be able to use the rem unit with a pixel fallback for any property. For example:

.remify(@Property, @value) {
@rem: (@value / 10);
~"@{property}": @value * 1px;
~"@{property}": ~"@{rem}rem";
}

@metaskills

This comment has been minimized.

Copy link

metaskills commented Mar 12, 2012

Should be closed? My understanding is that 1.3 added this feature?

@metaskills

This comment has been minimized.

Copy link

metaskills commented Mar 12, 2012

This is the commit 93b23d2 that added it and referenced in my updated blog post.

@sirlantis

This comment has been minimized.

Copy link
Contributor Author

sirlantis commented Mar 12, 2012

Nope, 93b23d2 added support for selectors (~".span@{n}") { color: red; } while this commit also allows them on properties ~"@{property}": red;.

However @cloudhead made it clear that he doesn't like 93b23d2 to begin with (being too much a preprocessing feature and breaking the language design). Thus I expect him not to merge this pull-request and rather reject/close it.

In the end that depends on which direction less.js will take with 1.4.

@metaskills

This comment has been minimized.

Copy link

metaskills commented Mar 12, 2012

Wow :(

@stefanpearson

This comment has been minimized.

Copy link

stefanpearson commented Mar 12, 2012

That's a shame. A perfect use case would be

.selector {
.rem("margin-bottom", 10);
}

CSS -

.selector {
Margin-bottom: 10px;
Margin-bottom: 1rem;
}

@remoun

This comment has been minimized.

Copy link

remoun commented Mar 13, 2012

I didn't know I could be used as a unit...

This looks cool, though. :-)

@Akkuma

This comment has been minimized.

Copy link

Akkuma commented Mar 15, 2012

If features like this don't make it into the core, isn't it only a matter of time someone forks it with the intent of allowing all preprocessor features?

@nadersoliman

This comment has been minimized.

Copy link

nadersoliman commented Mar 18, 2012

I like it ... thumbs up sirlantis

@jmgunn87

This comment has been minimized.

Copy link

jmgunn87 commented Apr 28, 2012

This would/will solve this problem using IMO a more in keeping syntax => #790

@borodean

This comment has been minimized.

Copy link

borodean commented May 1, 2012

I've made a set of mixins to workaround this issue: https://github.com/borodean/less-properties

It works like this:

@property: border-radius;
.class {
  .property(@property, 10px);
}

Which will output in:

.class {
  -less-property: property;
  border-radius: 10px;
}

Or even this:

.class {
  .property(border-radius, 10px, 'moz webkit o');
}

Which outputs in:

.class {
  -less-property: property;
  -moz-border-radius: 10px;
  -webkit-border-radius: 10px;
  -o-border-radius: 10px;
}

I would appreciate any help for developing this set further.

@rentalhost

This comment has been minimized.

Copy link

rentalhost commented May 12, 2012

+1

1 similar comment
@lucas-clemente

This comment has been minimized.

Copy link

lucas-clemente commented Jun 10, 2012

+1

@jmgunn87

This comment has been minimized.

Copy link

jmgunn87 commented Jun 12, 2012

@lazd

This comment has been minimized.

Copy link

lazd commented Jun 27, 2012

Either this or #790 would help LESS to be more DRY. Even Twitter Bootstrap is stuck repeating itself over and over because there is just simply no decent way to do what needs to be done:

.rotate(@degrees) {
  -webkit-transform: rotate(@degrees);
     -moz-transform: rotate(@degrees);
      -ms-transform: rotate(@degrees);
       -o-transform: rotate(@degrees);
          transform: rotate(@degrees);
}
.scale(@ratio) {
  -webkit-transform: scale(@ratio);
     -moz-transform: scale(@ratio);
      -ms-transform: scale(@ratio);
       -o-transform: scale(@ratio);
          transform: scale(@ratio);
}

I believe that this is one of the language features that is holding LESS back from having a complete set of CSS3 mixins like Compass.

@sirlantis

This comment has been minimized.

Copy link
Contributor Author

sirlantis commented Jun 27, 2012

@sashasklar @cloudhead Could you add an opinion on this issue? Is that something that you object to have in LESS? I understand your concert that this adds a macro-like mechanism to the language, however it's use is restricted and useful (see examples from above).

@sturobson

This comment has been minimized.

Copy link

sturobson commented Jun 28, 2012

Although a nice idea this shouldn't be implemented because adding prefixes carte blanche will add bloat to your code. Not every CSS3 element has had all the prefixes all the time. So adding them all to something would be a bad thing to do.

@sirlantis

This comment has been minimized.

Copy link
Contributor Author

sirlantis commented Jun 28, 2012

@sturobson This pull-request only allows the ~"@{foo}": @value; syntax and isn't a recommendation for .experimental mixin (which is just one of the multitude of use-cases).

Whether or not to add and an .experimental-mixin is up to developers of libraries/frameworks building upon less like bootstrap.

@jmgunn87

This comment has been minimized.

Copy link

jmgunn87 commented Jun 28, 2012

Personally the syntax is ugly with this fix. #790 will be alot easier to follow imo

@sirlantis

This comment has been minimized.

Copy link
Contributor Author

sirlantis commented Jun 28, 2012

@jmgunn87 This pull-request isn't about assigning variables, it's about allowing variables on the left-hand side of property rules. #790 solves a different problem.

@jmgunn87

This comment has been minimized.

Copy link

jmgunn87 commented Jun 28, 2012

no it doesn't. read it carefully. it is exactly the same as php's variable variable syntax. This is for having variable property names, not variable values.

@sirlantis

This comment has been minimized.

Copy link
Contributor Author

sirlantis commented Jun 28, 2012

#790 is about writing dynamically to a variable through another variable (variable variable names). The PHP equivalent: $$foo = $value;.

~"@{foo}": @value; from this pull-request outputs a CSS rule (and doesn't set a variable). The PHP equivalent: echo $foo.": ".$value;.

@jmgunn87

This comment has been minimized.

Copy link

jmgunn87 commented Jun 28, 2012

you can do the same thing. I've tried it. I will write a use case to show an example

@ice-horse

This comment has been minimized.

Copy link

ice-horse commented Sep 1, 2012

I was looking for a feature like that and hope it will be implemented soon. Would save a lot of time just being able to use the following instead of declaring mixins for every single CSS3 property:

.prefixify(@property,@params) {...}
@matthew-dean

This comment has been minimized.

Copy link
Member

matthew-dean commented Sep 5, 2012

I agree that this is pointing to a real problem: Having to repeat prefixes.

Is this the most elegant solution? Not sure. It is a solution, but the escaped string syntax always looks messy to me.

In addition, prefixing often looks similar, but not all prefixes are needed in every case. Some things get finalized (non-prefixed) in a browser before others, meaning that you would have to override / add prefixes in specific cases anyway.... meaning abstracting a "prefixing" function doesn't really make sense.

So....

@lukeapage

This comment has been minimized.

Copy link
Member

lukeapage commented Sep 5, 2012

Assuming I get my pull request to have selectors like this

.@{variable}-1 {
}

then the obvious extension is to use this for property names

-webkit-@{property}: my-stuff;

in dotless I was thinking of writing a plugin that replaced all property names ending in -all-browsers with duplicates for every browser prefix. e.g.

-all-browsers-X: Y;

becomes

-ms-X: Y;
-webkit-X: Y

etc.

So another question.. what uses does prefix interpolation have outside of prefixing of browser properties?

@lukeapage

This comment has been minimized.

Copy link
Member

lukeapage commented Feb 25, 2013

alternate idea for prefixes

#1199

only remaining case is this

.selector {
  .rem("margin-bottom", 10);
}

==>

.selector {
  margin-bottom: 10px;
  margin-bottom: 1rem;
}

which I'm not sure I like.. its a good abstraction, but yeh, not sure about it.

@demoive

This comment has been minimized.

Copy link

demoive commented May 26, 2013

Here's my solution to this problem: vendorify.less.

It was inspired by @borodean's solution, but from his test cases noticed it had a bug. For example, you can't pass in values with spaces nor does it support comma-separated compound values - which would be needed for things such as multiple backgrounds, and gradients, etc.) When I started going through his code to find a fix, I created what I think is a more simple solution to the same idea. Sure, it won't allow you to change which vendor prefixes get used (unless you modify the actual mixin), but how often do you need to specify different vendor prefixes within the same project?

Checkout the test cases in the comments below my vendorify.less Gist. It is quite extensive allowing you to specify values with spaces:

/* LESS */
.style {
  .vendorify(border-radius, 5px 10px);
}

/* CSS Output */
.style {
  -less-vendorify: ;
  -o-border-radius: 5px 10px;
  -ms-border-radius: 5px 10px;
  -moz-border-radius: 5px 10px;
  -webkit-border-radius: 5px 10px;
  border-radius: 5px 10px;
}

and even compound values:

/* LESS */
.style {
  .vendorify(background, url(bg2.png) repeat, url(bg3.png) no-repeat);
}

/* CSS Output */
.style {
  -less-vendorify: ;
  -o-background: url(bg2.png) repeat, url(bg3.png) no-repeat;
  -ms-background: url(bg2.png) repeat, url(bg3.png) no-repeat;
  -moz-background: url(bg2.png) repeat, url(bg3.png) no-repeat;
  -webkit-background: url(bg2.png) repeat, url(bg3.png) no-repeat;
  background: url(bg2.png) repeat, url(bg3.png) no-repeat;
}
@matthew-dean

This comment has been minimized.

Copy link
Member

matthew-dean commented May 26, 2013

So another question.. what uses does prefix interpolation have outside of prefixing of browser properties?

It's a good question. If it has none, then, like other similar issues, the syntax and problem should be focused on just the most elegant solution for that.

But.... I could see someone doing this?

.attach-red-border(@position) {
  border-@{position}: 4px solid red;
}

Seems weird, but rather than the theoretical, it's worth input on that point. Are people wanting this only for prefixes?

@jonschlinkert

This comment has been minimized.

Copy link
Contributor

jonschlinkert commented May 26, 2013

I've said this several times, the CSS spec is introducing custom properties, variable property names is a must-have that should be high on or priority list. Vendor prefixes is a convenient example (and not very compelling), but it's not the only use for custom properties.

@matthew-dean

This comment has been minimized.

Copy link
Member

matthew-dean commented May 26, 2013

Right. Even if it was only for prefixes, it still seems valuable to let the stylesheet author swap in property names. It seems like an intuitive addition. Let's do this, based on @lukeapage's syntax.

@jonschlinkert

This comment has been minimized.

Copy link
Contributor

jonschlinkert commented May 26, 2013

Which one? He's mentioned a couple, you probably mean @{variable}, so it's consistent with selector interpolation? If so I'm in agreement. The other one that comes to mind was geared towards the prefixes: -*-, so I'm guessing that's not the one you meant

@matthew-dean

This comment has been minimized.

Copy link
Member

matthew-dean commented May 27, 2013

Yes (to the first point). This seems like an easy win:

-moz-@{myvar}: value;
@Soviut

This comment has been minimized.

Copy link

Soviut commented May 27, 2013

Agreed. Variable interpolation within property names is a no-brainer. +1 for the border-@{attr}: value syntax.

@jonschlinkert

This comment has been minimized.

Copy link
Contributor

jonschlinkert commented May 27, 2013

Maybe this is implied, but to be clear the syntax should allow the entire property name to be interpolated:

@{prop}: value

And since values can already be variables:

@{prop}: @value;
@Soviut

This comment has been minimized.

Copy link

Soviut commented May 27, 2013

Again, agreed. There are plenty of situations where this would be handy, and just as many places it would be critical to work with mixins.

@jonschlinkert

This comment has been minimized.

Copy link
Contributor

jonschlinkert commented May 27, 2013

just as many places it would be critical to work with mixins.

Agreed. I think we don't take that for granted as often as we should.

@xaguilarf

This comment has been minimized.

Copy link

xaguilarf commented Sep 4, 2013

+1

@dwt

This comment has been minimized.

Copy link

dwt commented Sep 25, 2013

+1 Any way to write a generic and clean .vendorize is required - I actually don't care which.

I want something that allows me to generate vendor prefixes without the repetition, and allows me to override specific vendor names if they differ.

@gabssnake

This comment has been minimized.

Copy link

gabssnake commented Oct 15, 2013

+1 for the awesome power of:

@{prop}: @value;
@suparngp

This comment has been minimized.

Copy link

suparngp commented Oct 15, 2013

+1 for this,
Currently I am doing this to make it happen:


     qwerty: 1 e("; @{type}:") rgb(red(@color), green(@color), blue(@color));

This basically renders an extra property called qwerty which is ignored by the browser.

@fahad19

This comment has been minimized.

Copy link

fahad19 commented Oct 18, 2013

馃憤 would be awesome if this gets merged.

@Soviut

This comment has been minimized.

Copy link

Soviut commented Oct 19, 2013

Yeah, I doubt anybody wants to rush it into an already jam-packed 1.5 release, but if it can be done, that would be fantastic.

@badiezadegan

This comment has been minimized.

Copy link

badiezadegan commented Oct 21, 2013

a great feature for making right to left pages from left to right ones... 馃憤

@lukeapage

This comment has been minimized.

Copy link
Member

lukeapage commented Dec 19, 2013

later pull request merged.

@lukeapage lukeapage closed this Dec 19, 2013

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