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

Issues with the new :extend() in 1.4 beta #1226

Closed
normadize opened this issue Mar 15, 2013 · 14 comments
Closed

Issues with the new :extend() in 1.4 beta #1226

normadize opened this issue Mar 15, 2013 · 14 comments

Comments

@normadize
Copy link

First, I think the CHANGELOG.md is wrong as it says :extends() and &:extends(); -- note the extra s.

I managed to get input { &:extend(.button); } to work, but not this input:extend(.button); ... for the latter, I get:

C:\>lessc <path>\style.less
ParseError: Unrecognised input in <path>\style.less on line 730, column 2:
729     //input { &:extend(.button); }
730     input:extend(.button);
731 }

The commented line works fine. What am I missing?

@normadize
Copy link
Author

Also, the margin property in both of these is ignored:

input { margin-top: 2em; &:extend(.button); }
input { &:extend(.button); margin-top: 2em; }

A few more:

  • Using :extend() twice doesn't appear to work (the second one is ignored) ... is this not supported?

    input { &:extend(.button-primary); &:extend(.button-large); }
    
  • Linked to the above is :extend() supposed to support multiple classes? e.g. :extend(.button, .button-primary) or :extend(.button.button-primary) ... these appear like it struggles to work, but it doesn't get there ... especially if I try more than 2 classes.

  • it would probably be better (expected?) that :extend() also inherits pseudo classes from the parent element, i.e. ':hover,:focus`, etc. Currently, I have to manually add each of them with multiple extends, like:

    input { &:extend(.button); }
    input:hover { &:extend(.button:hover); }
    input:focus { &:extend(.button:focus); }
    etc
    

    Is there a syntax to fetch and inherit all of them?

In case I'm missing something, could someone enlightedn me with the proper syntax?

@lukeapage
Copy link
Member

Ok, it needs to be documented properly. for now you can refer to my blog post until I convert it into proper documentation http://www.scottlogic.co.uk/2013/03/extends-in-less/

other format is input:extend(.button) { input: rules; } e.g. like a normal selector

try all... &:extend(.button all);

and yes, comma seperated or multiple extends should work.

This works for me.. can you give an example that doesn't ?

.button-primary {
  color: black;
}
.button-large {
  color: white;
}
input { &:extend(.button-primary); &:extend(.button-large); }

@lukeapage
Copy link
Member

changelog updated, thanks.

raised a new issue about documentation. less/old-lesscss.org#73

Let me know if you find any more issue and anything specific we will raise as a new bug.

@normadize
Copy link
Author

Thanks for the tip, I'll read your blog and try the tips. As for the example, here:

.submit {
    &:extend(.button);
    &:hover:extend(.button:hover);
}

The above dioes with an error ParseError: Unrecognised input in <path>\style.less on line 773, column 2:, while this one (which is very similar to yours):

.wpcf7-submit {
    &:extend(.button);
    &:extend(.button-large);
}

does not issue an error, but the 2nd class (.button-large) is not extended (I checked the css output), which is what I reported the first time in my post above.

Either I'm missing something or this is still buggy. I'm seeing lots of stuff going wrong/unexpected ...

Shall I send you my less file so you can try yourself?

@normadize
Copy link
Author

I see you closed this, though I posted just above with the details that you asked ...

Also, if the extended class is defined after the call point, then it overrides the properties you set ... this (somewhat) unexpected and non-intuitive behaviour, and could be fixed:

submit:extend(.button) {
    margin-top: 2em;
}

.button is defined after that point so in a way it makes sense to override the margin-top property. However, LESS is able to parse and extend .button with .submit even though .button is defined afterwards ... I would say that a more expected behaviour would be for LESS to automatically move the actual definition of .submit (i.e. .button { margin-top: 2em; }) after all the definitions of .button.

Programming wise, it is expected that what you define inside the class overrides anything you extend ... at least that's what sounds normal to me.

@normadize
Copy link
Author

Just in case you missed it, I posted quite a few examples in my several posts above, which cause issues for the :extend() ... in case you haven't already, you could read my posts above in detail if you wish. I hope this helps.

@normadize
Copy link
Author

Here's another one:

.submit:extend(.button-primary all) {
    margin-top: 2em;
}
.submit:extend(.button-large all) { }
.submit { padding-left: 2em; padding-right: 2em; }

The above code are the last lines in the .less file. The padding properties however are still overridden by the extended classes .button-primary and .button-large because after replacement, there now exists .button.submit, which takes precedence ...

Again, I understand why, but it took me by surprise -- I could live with it, I'm not even sure this one is worth "fixing". I guess the user has to know the stylesheet pretty well or else this kind of thing is a killer.

@normadize
Copy link
Author

Another one, and I think I'll stop and, sadly, also stop using 1.4-beta and revert to 1.3.3 as I'm wasting too much time trying to get :extend() to work.

Consider the following definition, where the class .button-large appears only in conjunction with .button as:

.button.button-large,.button-group.button-large .button { height:30px;line-height: 28px; }

There is no other definition of .button-large in the file.

(A) This "appears" to work but looks odd, see below:

.wpcf7-submit:extend(.button-large all) {
}

(B) This doesn't work at all:

.wpcf7 {
    .wpcf7-submit:extend(.button-large all) {
    }
}

(C) This does work (.button is prepended)

.wpcf7 {
    .wpcf7-submit:extend(.button.button-large all) {
    }
}

The class .wpcf7-submit does reside inside .wpcf7.

The "failures" are odd. In the (A) case, the output css is:

.button.button-large,
.button-group.button-large .button,
.button.wpcf7-submit,
.button-group.wpcf7-submit .button {
  height: 30px;
  line-height: 28px;
}

While in the case of (B) I get:

.button.button-large,
.button-group.button-large .button,
.button.wpcf7 .wpcf7-submit,
.button-group.wpcf7 .wpcf7-submit .button {
  height: 30px;
  line-height: 28px;
}

And in the case of (C) it looks fine:

.button.button-large,
.button-group.button-large .button,
.wpcf7 .wpcf7-submit {
  height: 30px;
  line-height: 28px;
}

It's a tricky issue ... for now, I guess the user has to be specific and write the entire chain of classes inside :extend().

@lukeapage
Copy link
Member

.submit {
    &:extend(.button);
    &:hover:extend(.button:hover);
}

is not valid. Try

.submit {
    &:extend(.button);
    &:hover:extend(.button:hover) {}
}

though this doesn't behave as expected. I raised a bug about it here #1227

I am not sure I agree about changing the ordering. The extend does not alter the order of the source or the target.. surely that is more predictable? moving things around based on arbitrary parts of the selector path may work for you but not for other people, its too complicated.

In your second example the ordering won't make any difference if the selectors have different complexities.

I think you need to stop commenting, try to understand what less is trying to achieve with extends, and how you want to use them and if you find a full example of something that does not work that should do based on how we say extends work, raise a new bug, with a full example. I'm sorry but I am really struggling to follow... You can't just put the extend part of the less and say "it doesn't work" because we have unit tests that show that it does work in those scenarios. Take your time, give a full broken down example and I'll answer why it isn't giving the output you expect and then clarify whether it is a bug or a mis-understanding on your side.

@lukeapage
Copy link
Member

In your last comment, (a), (b), (c) are as I would expect

@normadize
Copy link
Author

I don't think all my examples were misunderstandings on my part, but I'll stop commenting as you asked. Good luck with the project.

@lukeapage
Copy link
Member

Sorry @normadize I was too harsh.. was just overwhelmed a bit - you definitely found one bug, so thanks for that. As I understand it, the rest of the points are just it not working as you expect so far. If you have that example of two extends not matching both, I would like to see it.

If you want to understand why it works the way it does and why I'm a tetchy about the behaviour, maybe have a read of the 160 comments on #1155

@matthew-dean
Copy link
Member

@normadize - Some back story: what we found is that basically every LESS developer had a different expectation of how an extend syntax would behave, sometimes a slight difference, sometimes a major one. So, I think what @lukeapage was trying to say is that it might help to read what different expectations were in that thread, and how those were reconciled (or were not reconciliable). It's more complex than it seems, and probably the most complex feature to ever be added into LESS.

Alternatively, you can wait until there's some documentation to guide usage, or use test cases created so far as examples to extrapolate from. If we're missing test cases, let us know.

@jonschlinkert
Copy link
Contributor

@normadize I'm actually working on updating the docs and website, so just a thought - if you'd like to contribute to the documentation for these new features it might be better use of your energy. we can always use help from passionate members of the community like yourself

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

No branches or pull requests

4 participants