-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Don’t recognize extendable selectors outside of @media contexts #456
Comments
Can I see a concrete use case instead of foos? Hunt & pecked on my iPhone... Sorry if it's brief! On Jul 23, 2012, at 1:19 AM, Scott Kellumreply@reply.github.com wrote:
|
line 45 and 53 here: https://github.com/scottkellum/Singularity/blob/master/stylesheets/singularitygs/_mixins.sass#L53 Having a compile choke in a |
I'm not convinced this a bug. If a user uses |
I think you guys need to be extremely explicit about how you want extends to behave and follow through on that. Totally understand this is a complex issue, but to me this is more confusing as a user to have something that is isolated pop up a warning for matching code in explicitly different contexts. RWD requires flexibility of multiple contexts and extend is the only way to consolidate selectors attached to styles. It is extremely difficult to implement extends consistently in a RWD context. |
We have a very clear idea of how we want In this case, we want In general, I think it's a good idea to print errors when the actual behavior can't follow the expected behavior. This protects users who expect behavior consistent with the espoused semantics of |
Sorry, maybe I didn’t make that clear. I do not want this behavior: Well, I am fine with that behavior, but the issue here is that the new behavior is not consistent. If the contexts are explicitly different then: If I explicitly place styles inside a context to extend I should be able to extend them, no? I shouldn’t have to change my whole naming scheme just to extend these. I too can’t wait until this comes to browsers, but can we at least make it a somewhat useful feature in Sass? When used correctly it can save so much file size. |
I find it's actually quite hard to talk about these things using abstract class names like .foo {
@extend %screen-foo;
a: b;
}
@media screen {
%screen-foo {
c: d; //presumably this would have it's own properties.
}
.bar {
@extend %screen-foo;
e: f; //presumably this would have it's own properties too.
}
} |
Awesome, Thanks Chris. I will try plugging this in when I have a moment to really dive in. Thinking of a few more ways around the issue as well using Sass::script. |
The problem with this is that it's still pretty likely that the user intended the |
We're running into this deprecation warning and issue with Compass sprites. For example this SCSS: @import "sprites/*.png";
.icon {
@include sprites-sprite(regular-icon);
}
@media (max-width: 768px) {
.icon {
@include sprites-sprite(small-icon);
}
} Will give you:
Also if you include the sprite only in the media query and not at the top level it doesn't generate the class at all in the CSS and the image doesn't work. Maybe we're just doing it wrong! EDIT: Never mind, this should probably be a Compass issue not a SASS issue. See Compass/compass#617 I'll leave it here in case anyone else ends up here researching the same issue. Using this mixin fixed the issue for us: https://gist.github.com/3105369 (and my SCSS port: https://gist.github.com/3223921) |
While I find this behavior annoying, I still think it's the lesser of several evils. Closing. |
I am new to SASS and I just learned and saw the power of %placehoders. This is a much better way than using mixins for repeating code. Today I first saw this Warning "@extending an outer selector from within @media is deprecated" and I was very disappointed because of the mentioned "This will be an error in Sass 3.3". If one uses it the wrong way it will not work, but if someone knows what he is doing, this is a great way to reduce size of compiled CSS. By turning the warning into an error, you protect the bungling at a very high cost, by limit the gifted. You asked for some example code. I made a mixin recently which I wanted to discuss. Let's see, if I am gifted or should be protected ;-) @mixin banner-bg($height, $bgColor, $opacity:null, $extends:null) {
&%xxx{
height:$height;
background-color: $bgColor;
@if $extends != null {@extend #{$extends} }
}
@extend %xxx;
@extend %bannerbg;
&:before{
@extend %xxx;
@if $opacity != null {@include opacity($opacity);}
content: "";
position: absolute;
right: 0;
left: 0;
z-index:-100;
}
} The Placeholder %xxx causes this strange warning, although it is 100% sure that the two elements that use that Placeholder will be in the same @media section. I do not understand why this should be prohibited. |
Every time you include this, you're re-using the same However, reliably creating a unique identifier is hard, so I had to write some ruby to do this. Here is your code, working without warnings and without relaxing our error condition: All that said, I think this is compelling enough to add a unique identifier generation ability to Sass so I've created #771 to track that. |
OK, when I wrote my comment I was pretty sure, that someone would proof that I am bungling and not gifted ;-). I have some comments still, I write them over there. |
Note: In #774 we have proposed a new directive ( |
I used @mixin to resolve this warning:
Not sure if it's the best approach but thought I would share. |
This behaviour is preventing us from what I see as a valid use case for using @extend within a media query. We are using Glue (with some modifications to generate a scss file instead of css) and style our icons up like this:
Now, this works fine... Until you need to show a different icon for a different viewport size for example:
Is there anything we can do to work around this? I tried the mixin suggested by @ntreadway above but that didn't seem to work. |
@scottkellum, @WillsB3, it seems that Sass maintainers just don't want us to be able to do that. :( I also tried to bring this problem up (i'm sorry for creating a duplicate issue). I suggested that Sass should maintain a separate extendable list of selectors for each unique media query. But @chriseppstein considers this to be counter-intuitive for many users. @chriseppstein points out that Sass does allow to extend from media queries. To do this, you have to declare a separate extendable selector for each unique media query, so that the extended value wasn't found outside the media query. This can be done either manually or using a technique called "Maps, Media & Extend, Oh my!". Basically, the "Oh, my" technique requires that:
Oh my, it's so intuitive that i'm still not sure whether i correctly understand what's going on in that example. So you can correct me if you grasped it better. Well, this is the direction where modern Sass is heading for. Another example of the tendency is the way parent selector can be interpolated. In LESS and Stylus you can do: .fieldset {
background: red;
&-field {
background: blue;
&-label {
background: green; }}} To achieve the same result in Sass, you have to do: .fieldset {
background: red;
@at-root #{&}-field {
background: blue;
@at-root #{&}-label {
background: green; }}} I guess that if you choose Sass over other preprocessors then you just have to accept that "intuitive" means not "intuitive from the perspective of users" but rather "from the perspective of the internal logic of the Sass compiler". |
It's very easy to come up with individual examples to justify all kinds of behavior, but that doesn't mean the behavior is good for the ecosystem in general. This issue in particular is thorny because the ideal behavior just isn't possible for a preprocessor. The semantics of There's been a lot of discussion about this, here and elsewhere, but really what it all boils down to is "I wish I could do it." I wish that too. But it's not enough to want it; it has to be possible. In all this discussion, I've seen practically no one* address the issues preventing us from making this work. If you want to use * With the notable exception of @rachelnabors who suggested the current scheme of allowing |
@nex3 Say, a user wants to extend from a media query. Consider @WillsB3's use case as an example. As extending from media queries is impossible, what workaround will he use? What does everybody use in this situation? Mixins. It's the only thing that works. I find these in all my projects: =background-foo
background: url(foo.png) pink
%background-foo
+background-foo This looks redundant and it is. But i'm forced into having both mixins and extends implementations of snippets because i need them inside media queries. The mixins workaround produces a CSS much larger than the proposed functionality of extends "silently multiplying the size of their selectors", because each mixin usage will have a duplicate rule in CSS, whereas each extend will only have a single duplicate per media query. Say, you have three different media queries and you're trying to extend four times in each of them and four times outside media queries. The suggested extends approach will produce 4 CSS rules and the mixin workaround will produce 13. So basically:
Please reconsider. PS There's of course the "Oh, my" workaround (see above), but it does absolutely the same: producing a unique extendable for each media query. It's the same what we ask here, but with absolutely ridiculous usage complexity. |
@lolmaus The issue isn't just the output size in an absolute sense; in general, people tend to overestimate the impact of duplicated text and underestimate the effectiveness of gzip compression_. The primary issue is the ability of users to reason about what their output will look like. Both the mixin solution you describe and Chris's solution have the extremely valuable advantage of _making the functioning of the compiler explicit*. When a user writes You're also underestimating the degree of extra text that would be generated if we silently copied selectors. In addition to the selectors being extended, the full media query would need to be copied in order to preserve the order that the selectors were defined in (which is clearly important). For example: %foo {color: blue}
.foo {@extend %foo}
.bar {color: red}
@media screen {
.baz {@extend %foo}
.qux {color: green}
} would need to be compiled to .foo {color: blue}
@media screen {.bar {color: blue}}
.bar {color: red}
@media screen {.qux {color: green}} * This quirk of estimation means that generating more output than users expect will make them very unhappy, but generating large amounts of output that they do expect won't cause many problems. |
Oh come on! Sass But there's more than that. I believe that most users just don't even notice that their incorrectly extended selectors become bloated, simply because it doesn't break the looks. So my point is: if you use extends, you should either get your ass to learn how it works or just accept the fact that the convenience of using Sass 3.3 introduces a lot of new stuff, some of it is far from being intuitive. Why do you expect that common users will be quick to grasp I believe that if Sass 3.3 introduces an ability to extend from within media queries, it will not make anyone unhappy with the size of their CSS. If a user is advanced enough to care about the size of generated CSS, it won't be hard for him to learn how Sass 3.3 extends work. PS Ain't Sass 3.3 capable of merging identical media queries yet? Is there an issue ticket for that already? |
@lolmaus There is a Latin phrase that I think sums up the counter-point to your argument: Ignorantia juris non excusat or "Ignorance of the law does not excuse". Suggesting that it's OK to codify bad behavior because people don't know any better is a terrible suggestion made worse by the rude delivery. I, and many fellow framework developers, know exactly how As for your tangent into other features of Sass 3.3, while One of the key aspects of User Experience (of which Developer Experience, what we're talking about, falls under) is that actions should have predictable outcomes. On a website, if a red button had the text "Free Shipping" and if clicked on a product page, it provided free shipping, but if clicked on from an item in a carousel shipping was $5 because somewhere in the business logic of that button it's only free from a product page, people would be pissed off. The argument is for making the "Free Shipping" button always mean "Free Shipping" and if shipping isn't free, make sure the user explicitly knows. |
Speaking of Free Shipping, @nex3 @chriseppstein thoughts on providing a way for users to explicitly state that they wish a new extension context be created (i.e. duplicate the thing being extended and use that where appropriate)? Something like the following: %full {
width: 100%;
float: left;
clear: both;
}
%half {
width: 50%;
float: left;
}
.sidebar-1 {
@extend %full;
background: blue;
@media (min-width: 500px) {
@extend %half !duplicate; // A new extend context will be created here for the all, min-width: 500px media context and extension will work as normal.
background: red;
}
}
.sidebar-2 {
@extend %full;
background: green;
@media (min-width: 500px) {
@extend %half !duplicate; // Because a context for this exact media query already exists, it will extend that one instead of creating a duplicate context
background: orange;
}
}
.content {
@extend %half;
background: purple;
} would compile to .sidebar-1, .sidebar-2 {
width: 100%;
float: left;
clear: both;
}
.content {
width: 50%;
float: left;
}
.sidebar-1 {
background: blue;
}
@media (min-width: 500px) {
.sidebar-1, .sidebar-2 {
width: 50%;
float: left;
}
.sidebar-1 {
background: red;
}
}
.sidebar-2 {
background: green;
}
@media (min-width: 500px) {
.sidebar-2 {
background: orange;
}
}
.content {
background: purple;
} Of course the optimization around this would be difficult, needing to ensure that the matching only happens for identical |
Maybe you didn't undersand what i'm suggesting and that's why we have this argument... I'm suggesting that a new extend context should be unconditionally created for every unique media query type. Within each such context, This behaviour is absolutely predictable, it does not require Sass to do any decision making, it does not require rearranging selectors "to the first or last item", and the "normal
If you consider this behaviour bad, please provide concrete examples and arguments. I provide mine below.
Currently Sass does not allow to extend the same selectors inside different media queries (or inside + outside a media query). It just doesn't work, and this fact has no benefit at all.
I never did such an argument. My argument was: Let users do what is currently impossible. If they want to use it, they can read docs and learn how it will work for them. If they're neglectful so much that they don't read, then they will hardly notice the difference. @nex3 cares about how users will be surprised by the change, but they are already misusing Let me empathize this. Nobody is currently extending same selectors within different media queries because it just doesn't work! Enabling this will not change the output of existing code. Only people who read that extending same selectors from different media queries has been enabled in Sass 3.3 will start using it. And they will know how it works because they have just read it! There also will be newbies who will ignorantly use extends in media queries without knowing how it works. But it will not hurt them because:
So why do people stumble on it all the time? Parent selector is probably the most misused feature in Sass 3.2. "Why does my As for I just don't understand why you let that counter-intuitive stuff pass and ban the possibility of extending from different media queries. It just does not make sense. :(
The
Because doing this without .foo
@media (min-width: 500px)
@extend %foo !yes-i-really-mean-it
@extend %bar !please-master-i-really-need-to-do-an-extend-from-this-media-query
@extend %baz !i-ploofing-know-what-i-m-doing-you-sparkling-sunflower |
@lolmaus Please keep your language civil. I've edited out your profanity, and further cursing will result in me deleting your comments entirely. These discussions are public and I won't have them taking a tone that makes the community looks like a bunch of squabbling adolescents. Having The idea that users already have trouble understanding how @Snugug Adding a flag for this behavior is certainly one way to allow a back door for users to be explicit. Feel free to file a separate issue about it where we can discuss it further. |
@nex3 I fully agree to what you said, there is nothing worse than a machine that does something because a User made a mistake. It should never ignore errors and keep the user thinking he did everything right. Meanwhile the Machine would have to do something that it thought that the user wanted to have. But How should the machine know? |
Sass 3.2 is throwing this error:
for this code:
It seems like Sass should be ignoring the selectors outside the
@media
context for this new@extend
behavior and result in no errors.The text was updated successfully, but these errors were encountered: