Bug 56727

Summary: Implement experimental mixins feature.
Product: WebKit Reporter: Shane Stephens <shanestephens>
Component: CSSAssignee: Shane Stephens <shanestephens>
Status: RESOLVED WONTFIX    
Severity: Normal CC: bfulgham, daniel, dglazkov, eric, hyatt, joepeck, kangax, komoroske, mathias, mikelawther, paulirish, peter, phiw2, sam, simon.fraser, tabatkins
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch none

Description Shane Stephens 2011-03-20 18:23:42 PDT
Implement experimental mixins feature.
Comment 1 Shane Stephens 2011-03-20 18:45:34 PDT
Created attachment 86289 [details]
Patch
Comment 2 Sam Weinig 2011-03-20 19:20:33 PDT
What spec is this from? How does this differ from the CSS Variables that Hyatt added a while back?  Why do we want this?
Comment 3 Shane Stephens 2011-03-20 20:04:52 PDT
This patch implements one of the CSS extension ideas that Tab Atkins presented recently (see slides here: http://www.xanthir.com/talks/2011-01-12/slides.html, blog post here: http://www.xanthir.com/blog/b49w0).  A formal specification is in progress. Mixins exist as a feature of both sass and less.

Mixins are complementary to CSS Variables - they provide sets of properties that can be bulk-imported into style definitions (see LayoutTests/css3/mixins.html in the patch for examples of usage), while Variables provide javascript-accessible values that can be used when defining property values.

Mixins allow common sets of properties to be defined once and used in multiple places.  They're useful for reducing the size of CSS sheets and improving modularity of CSS declarations. One of the clearest answers to why we want this is that mixins are a very commonly used feature in the sass and less pre-processors, indicating that they're a popular idea with web developers.
Comment 4 Daniel Glazman 2011-03-21 08:31:10 PDT
(In reply to comment #3)

> This patch implements one of the CSS extension ideas that Tab Atkins
> presented recently (see slides here: http://www.xanthir.com/talks/2011-01-12/slides.html,
>blog post here: http://www.xanthir.com/blog/b49w0).  A formal specification is
> in progress. Mixins exist as a feature of both sass and less.

Let's clarify, please...
A "proposal" is in progress. Not a specification... Tab has only proposed
this once to the WG and the WG has not worked on this proposal. So your
implementation is for the time being 100% experimental and the feature is
not discussed yet. Mixins are not yet on the radar of the WG.

Daniel Glazman
CSS WG Co-chair
Comment 5 Tab Atkins 2011-03-21 09:31:03 PDT
Daniel's got it right.  I haven't yet made any proposal to the CSSWG, though I plan to do so in the very near future.  I have too many specs on my plate.  ^_^
Comment 6 Sam Weinig 2011-03-21 10:49:44 PDT
If this is complementary to CSS Variables, it seems like getting CSS Variables back up and cleaned up and shippable would be a much better first step than inventing something new.
Comment 7 Shane Stephens 2011-03-21 12:49:38 PDT
Daniel: Ah sorry, I did not want to give the impression that this was something the CSS WG was currently specifying or working on - I should have said "a draft specification is in progress".

Sam: Work to add variables is currently in progress, by other coders.  As far as I can tell, this patch doesn't complicate their task.  Is there a particular reason to wait for variables to be committed first before discussing this feature?  I think that mixins are complementary in the senses that mixins and variables can happily co-exist, that each is valuable independently of the other, and that having both available provides multiplicative benefits.
Comment 8 Dimitri Glazkov (Google) 2011-03-21 13:06:30 PDT
(In reply to comment #7)
> Daniel: Ah sorry, I did not want to give the impression that this was something the CSS WG was currently specifying or working on - I should have said "a draft specification is in progress".
> 
> Sam: Work to add variables is currently in progress, by other coders.  As far as I can tell, this patch doesn't complicate their task.  Is there a particular reason to wait for variables to be committed first before discussing this feature?  I think that mixins are complementary in the senses that mixins and variables can happily co-exist, that each is valuable independently of the other, and that having both available provides multiplicative benefits.

Since this is an experimental feature, first good patch would be to add a flag for it. I think we also need dhyatt's/smfr's blessing that this is not a totally cockamamie idea. Perhaps even a webkit-dev mail?

Also, please try to keep patches smaller. Breaking this up into more reviewable pieces would be greatly appreciated.
Comment 9 Shane Stephens 2011-03-22 03:00:29 PDT
Created attachment 86450 [details]
Patch
Comment 10 Shane Stephens 2011-03-22 03:03:52 PDT
The new version of the patch hides the feature behind a flag (css-mixins).

I've been thinking about how to split this up.  One problem is that the natural split - implementation vs. inspector changes - leaves the inspector in a state where it crashes when the user performs certain actions (for example disabling a mixin).  This is why I implemented the inspector changes as well, so that I wouldn't leave WebKit with crash bugs.

Being behind a flag, however, it might be OK for there to be inspector crashes if one patch were submitted but the other weren't. What's the general approach for this sort of thing?
Comment 11 Dimitri Glazkov (Google) 2011-03-22 09:33:09 PDT
(In reply to comment #10)
> The new version of the patch hides the feature behind a flag (css-mixins).
> 
> I've been thinking about how to split this up.  One problem is that the natural split - implementation vs. inspector changes - leaves the inspector in a state where it crashes when the user performs certain actions (for example disabling a mixin).  This is why I implemented the inspector changes as well, so that I wouldn't leave WebKit with crash bugs.
> 
> Being behind a flag, however, it might be OK for there to be inspector crashes if one patch were submitted but the other weren't. What's the general approach for this sort of thing?

I think it's not great for inspector to crash :)

But the flag impl looks ok. Any luck summoning smfr/dhyatt?
Comment 12 Dave Hyatt 2011-03-22 10:57:16 PDT
I think this needs discussion on www-style first before we commit anything. If there's one lesson I learned from the CSS variables debacle and the datagrid debacle, it's not to commit code too early even if it's behind ifdefs.  

There's no reason to rush here.  Let's talk about this idea on the mailing list.

I will say that my old CSS variables proposal supported mix-ins using variables syntax without the need to invent something separate/new, so my first impulse of course is to ask why this has to be treated like a different feature with a different syntax.
Comment 13 Shane Stephens 2011-03-22 17:06:37 PDT
I agree that waiting and seeing how the discussion on www-style pans out is a good idea.  Here's a link to the thread for anyone who is interested: http://lists.w3.org/Archives/Public/www-style/2011Mar/0478.html.

I'm afraid I don't understand how your variables proposal supports mixins. Is this: http://disruptive-innovations.com/zoo/cssvariables/ the proposal you implemented?:

How would this approach support something like:

@trait colorMe {
  color: green;
  background: black;
}

.myClass {
  @mixin colorMe;
}

I guess you could define variables for color and background, and use them everywhere that the mixin is included, but this quickly gets unwieldy and difficult to maintain as a change to the mixin (for example by adding a border) would require finding and updating every style rule that uses the mixin.

Alternatively you could define the mixin as a class and add class attributes to each dom element that matches a style rule that you want the mixin to apply to.  Again this is somewhat unwieldy and additionally requires style-specific modification of the HTML source.

Although this patch doesn't support it yet, one of the ideas in Tab's proposal to www-style is to allow parameterization of traits.  Here's a strawman syntax:

@trait styleMe($fg, $bg) {
  color: $fg;
  background: $bg;
  border: 2px solid $fg;
}

This allows complex sets of styling to be collected together then applied flexibly within a document.  I don't see a nice way that you can do this with variables, as the source of variability is the set of @mixin declarations:

.class1 {
  @mixin styleMe(red, green);
}

.class2 {
  @mixin styleMe(white, black);
}

etc..
Comment 14 Dave Hyatt 2011-03-26 00:19:46 PDT
It wasn't in the proposal... it was in the implementation in WebKit. It basically looked very similar to your mix-ins stuff.  Just replace:

@trait colorMe {
  color: green;
  background: black;
}

with

@variable colorMe {
  color: green;
  background: black;
}

My main point is just that I don't really see any difference between CSS Variables and Mix-Ins.  I just think of them as the same feature.  Mix-ins are just variables where the value is a declaration block.
Comment 15 Shane Stephens 2011-03-28 20:08:09 PDT
We'd like to also support parameterization for mixins. For example, 

@trait styleMe($fg) {
  color: $fg;
}

.class1 {
  @mixin styleMe(red);
}

If @variable represented both values and sets of properties, then the following case is weird:

@variable styleMe($fg) {
  color: $fg;
}

@variables {
  foo: {color: red;}
  foo2: red;
}

.class1 {
  @mixin styleMe($foo, green); // not OK!
  @mixin styleMe($foo2, green); // OK!
}

@variables representing sets of properties could not ever be used as parameters - it seems they're a different concept.
Comment 16 Eric Seidel (no email) 2011-10-24 11:48:13 PDT
Curious what's the status of this bug?  Is the patch abandoned?
Comment 17 Brent Fulgham 2022-07-12 14:28:21 PDT
This seems to be abandoned.