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

Request for "rule replacement" feature #1372

Closed
creocoder opened this issue Jun 15, 2013 · 34 comments
Closed

Request for "rule replacement" feature #1372

creocoder opened this issue Jun 15, 2013 · 34 comments

Comments

@creocoder
Copy link

For example we have bootstrap/typo.less:

.page-header {
  padding-bottom: (@baseLineHeight / 2) - 1;
  margin: @baseLineHeight 0 (@baseLineHeight * 1.5);
  border-bottom: 1px solid @grayLighter;
}

and typo.less:

.page-header {
  color: #fff;
  background: #ccc;
}

and site.less:

@import "bootstrap/typo.less";
@import "typo.less";

Overall css result will be:

.page-header {
  padding-bottom: 9px;
  margin: 20px 0 30px;
  border-bottom: 1px solid #eee;
}

.page-header {
  color: #fff;
  background: #ccc;
}

My suggestion is to add new feature replace previous. For example bootstrap/typo.less:

.page-header {
  padding-bottom: (@baseLineHeight / 2) - 1;
  margin: @baseLineHeight 0 (@baseLineHeight * 1.5);
  border-bottom: 1px solid @grayLighter;
}

and typo.less:

!.page-header {
  color: #fff;
  background: #ccc;
}

and site.less:

@import "bootstrap/typo.less";
@import "typo.less";

Overall css result should be:

.page-header {
  color: #fff;
  background: #ccc;
}

This will be very useful for customizing css frameworks like twitter bootstrap without modifing its code.

@creocoder
Copy link
Author

@jonschlinkert I'll try to explain more what i want reach. I want to work with bootstrap with folowing workflow:

css/bootstrap/*.less <- will never touch them
css/my-styles.less <- all work going here

Now for example I want to change .page-header style. What i do. In my-styles.less i want:

@import "...all bootstrap lesses...";

!.page-header {
    ... //my owne styles here
}

Result css should be:

.page-header {
    ... //my own styles here, no bootstrap styles for .page-header from type.less, also there is should not be second .page-header in css
}

I'm not sure about syntax, we can discuss this. But this is should be something like:

[kill_upper_css_class_dublicate_with_that_style_symbol].class {
   //class exclusive css pops
}

@jonschlinkert
Copy link
Contributor

What keeps coming to mind as I think about this request is minimatch patterns. Imagine how much more powerful this concept would be if we modified the syntax of this feature in a way that (elegantly) allowed the usage of globbing patterns for selectors lol! My point is that ! is just an exclusion pattern, but imagine having the ability to use minimatch for specifying selector "patterns", rather than specific selectors.

@creocoder
Copy link
Author

Imagine how much more powerful this concept would be if we modified the syntax of this feature in a way that (elegantly) allowed the usage of globbing patterns for selectors lol!

I think its really great idea. Can you show some examples how it can be integrated?

@jonschlinkert
Copy link
Contributor

Sure, so the way it would work is that instead of just using !.alert-info, and !alert-success and then repeating that for each modifier or related class, you could instead use something like !alert-*, which would apply to all modifiers of the alert class. Perhaps even something like !.alert/** which might imply that all child selectors of .alert should be excluded.

@jonschlinkert
Copy link
Contributor

I'm saying "excluded", but I also consider "replacement" to be implicit.

@creocoder
Copy link
Author

So we can do for example:

!h*;

h1 {
   ...
}

h2 {
   ...
}

...etc

Doing !h* we say LESS to exclude all headers for example from upper includes and write own styles for that. Right?

@jonschlinkert
Copy link
Contributor

Yes, but you would also exclude any other selector that starts with "h", such as header, hgroup, html, hr etc. and presumably that would also include any "selector groups" or child selectors of those selectors.

This is completely fine though IMO, anyone who uses globbing knows this, so you use patterns that are intended to be specific enough to target what you need, but not so general that will accidentally target something else.

@creocoder
Copy link
Author

@jonschlinkert Seems its very powerfull. Its can also work this way:

!h*(color, border-*);

Mean not exclude all h*, but exclude only color property and border-* properties from that upper defined styles.

@jonschlinkert
Copy link
Contributor

Well, you would probably do something like:

h* {
  !color;
  !border-*;
}

But what I didn't mention is that minimatch also allows for inclusion patterns. What that means is that when combined with the @import (reference) directive, we have a very, very powerful system for controlling what we want or don't want from external libs. If you haven't read about reference it allows you to import an external lib, but only render the styles that are either mixed in, extended or otherwise "referenced" in your styles.

With your concept (and minimatch), we could do this:

@import (reference) "bootstrap.less";

.alert* {}
.nav* {}
.modal*  {
    max-width: 720px;
}
// etc.

Here, we're using "inclusion" patterns, which IMO is more powerful than exclusion.

@jonschlinkert
Copy link
Contributor

Did that example makes sense? I mean are you familiar with the "reference" feature?

@creocoder
Copy link
Author

@jonschlinkert Yes, i totally got idea.

@Soviut
Copy link

Soviut commented Jun 16, 2013

As usual, I'm going to interject and suggest possibly more appropriate terminology. I think that override may be a clearer term than replace in this situation, but I'm also doubting myself a little. What do you guys think?

@jonschlinkert
Copy link
Contributor

Override is correct, I got hung up on "replace" as well. But actually the original request was for both overriding and exclusion. So replacement probably seemed convenient as a catchall.

I would now probably describe this as a request for "the ability to use inclusion and exclusion patterns for greater control over which selectors are rendered when importing external libs".

Agreed?

@Soviut
Copy link

Soviut commented Jun 16, 2013

Agreed. I could see people asking "how can I override styles from an external library?" so I think the term is more likely to make sense and get hits when people are searching for help.

@lukeapage
Copy link
Member

This is the inverse of using reference and extend.. which is something I would probably suggest over rule replacement.. e.g. pick out the things you need and leave the rest rather than taking it all and then over-riding the bits that don't make sense.

I'd like to see how people use reference and extend and how libraries adapt before implementing more features to help using them.

@matthew-dean
Copy link
Member

I think with the fact you can reference styles in a root library (without outputting), and that you can name your classes something different and import what you want means that this doesn't seem necessary. The fact that you can't name your own classes to begin with is a problem created by the LESS library, not the parser, so I don't think it's our business necessarily to solve problems created by libraries (since they can be solved by re-architecting the library).

@jonschlinkert
Copy link
Contributor

@lukeapage agreed. I think that's important. I also think this request may be useful in the future so it will be fun to see how the dialog evolves here.

@matthew-dean, yeah I was afraid folks wouldn't completely get this request. It has nothing to do with "other" libraries or solving anyone's problems. The lib could by yours intentionally created for this purpose... ;-)

Just give this one the benefit of the doubt, I think there is more to this than will be immediately understood, not because it's "complex" or anything like that, but that there is no easily explained frame of reference from which to explain the feature. It will make more sense after @import (reference) is officially released and folks get experience with it.

@matthew-dean
Copy link
Member

@jonschlinkert What I mean is that the "rules" could be defined as mixins or variables, which was what was in my head.

So, you could do this:

.error-box-definition() {
  background: red;
  color: yellow;
}
// some other selectors.less file would have maps
.error-box {
  .error-box-definition;
}

Or, alternatively:

// selectors.less
@errorBox: ~".error-box";

// definitions.less
@{errorBox} {
  background: red;
  color: yellow;
}

@jonschlinkert
Copy link
Contributor

I also didn't get what he was asking for initially (to the degree that I actually asked if I could delete my comments lol, which was a first, but I didn't want to take away from the request), I suggest reading back through. It's not even close to what you're suggesting.

@matthew-dean
Copy link
Member

Including particular selectors from a library? I've looked through it; it still doesn't seem necessary. Rather than having inclusion patterns like .alerts* {}, a library could be grouping selectors by mixin and switching by variable. Or with reference, grouping such selectors by feature (less file), and allowing an author to include or simply reference.

I think there's more coverage than we may realize by existing or upcoming features, but the patterns have not emerged yet from usage. So, in general, I'm in agreement with Luke:

I'd like to see how people use reference and extend and how libraries adapt before implementing more features to help using them.

@matthew-dean
Copy link
Member

In other words, the assumption may be that this is needed based on how libraries are structured now, but they may restructure around use of reference, extend, and variable interopolation in selectors.

@jonschlinkert
Copy link
Contributor

I'm sorry, but I'm using reference a lot with both individual components and "manifests" that refer to libraries of components, and this request would be amazing to see implemented at some point. I'm quite familiar with our existing features and nothing can do anything like this. Reference, mixins and extend all require you to target specific selectors for inclusion, this request on the other hand would allow you to exclude selectors. And with the introduction of the minimatch, you could include or exclude any given pattern of selectors at once.

@creocoder
Copy link
Author

Guys. Cant got one thing. For example there is no that issue. But 1.5 released.

For example we have bootstrap/typo.less:

.page-header {
  padding-bottom: 20px;
  margin: 30px;
  border-bottom: 1px solid #ccc;
}

And my.less:

@import (reference) "bootstrap/typo.less";

// What should be here to get in result css only one .page-header rule and its definition:
// .page-header {
//     padding-bottom: 20px;
// }
//

?

@matthew-dean
Copy link
Member

But, Jon, how is a typical user going to know what exclusion patterns to use on, say, a library like Bootstrap, that won't have knock-on effects when they start applying some of the classes? Would it not be better to funnel library makers into creating a type of API architecture where those exclusions are predefined using mixin wrappers? So that people don't end up excluding things accidentally that break the library?

In the end, I don't disagree that we don't necessarily cover exactly what you're saying. I'm just wondering if that isn't, in fact, a good thing.

But, devil's advocate, I suppose class overriding is no less disruptive to code or dangerous than overriding properties (Issue #1342). So, I suppose it may be hypocritical to say that powerful features that are potentially destructive or confusing in some cases shouldn't exist. If the author is willing to take the risk, I guess it's somewhat on them.

But, just to bring it back to Luke's point, I think it's sound judgment to just slow a bit while people start building extend, reference, and variable interpolation in selectors into their projects and libraries. That way they may choose simpler, less destructive features first before using something more destructive in case they need it.

@Soviut
Copy link

Soviut commented Jun 19, 2013

The way I'm interpreting this rule is basically to have "cascading LESS". Where LESS files are aware of their structure and cascade rules in much the same way CSS does already. I see this as a natural extension, but one that must be planned carefully given the extensions that already exist (mixins, especially).

I think that namespaced variables are probably the first step in this direction, allowing libraries to both isolate and allow variable overriding without having to pollute a global scope. This takes care of many of the margin and colour issues one faces with Bootstrap, without the need for structure awareness; As long as Bootstrap is being imported in its LESS form.

However, namespaces and variables do not address the possibility of needing to import raw CSS and perform overrides when there are no variables or mixins to contend with.

@jonschlinkert
Copy link
Contributor

how is a typical user going to know what exclusion patterns to use

I was suggesting that minimatch is a much more powerful tool for inclusion. But there is still a need for selective exclusion.

@lukeapage
Copy link
Member

Guys. Cant got one thing. For example there is no that issue. But 1.5 released.

For example we have bootstrap/typo.less:

.page-header {
padding-bottom: 20px;
margin: 30px;
border-bottom: 1px solid #ccc;
}
And my.less:

@import (reference) "bootstrap/typo.less";

// What should be here to get in result css only one .page-header rule and its definition:
// .page-header {
// padding-bottom: 20px;
// }
//
?

.page-header:extend(.page-header) {}

http://www.scottlogic.co.uk/2013/03/extends-in-less/

@jonschlinkert
Copy link
Contributor

I just wanted to revisit this after working on a project where I really could have used it.

I just used :extend() on Bootstrap's .dropdown-menu class, which is pretty extensive, but there were only a couple of styles I didn't want to inherit. It would have been awesome to use minimatch exclusion patterns here.

Going even further, IMO it would make more sense to use minimatch than all. You could just do something like:

// Extend Bootstrap's .dropdown-menu
.dropdown-inverse {
  &:extend(.dropdown-menu *);
}

and with exclusion patterns

.notification {
  &:extend(.alert *, !.alert-foo);
}

@seven-phases-max
Copy link
Member

@jonschlinkert The problem is that these symbols conflict with existing CSS selectors, e.g.:

.alert * {
    color: red;
}

.notification {
  &:extend(.alert *);
}

! will also possibly appear within CSS4 selectors.

@bartelsmedia
Copy link

+1 Would love to see a way to exclude declarations, too.

E.g. Bootstrap has many declarations which may not be used by many but are compiled by all of the billion bootstrap users. As a consequence, all visitors of all bootstrap-built websites have to load this .css bloat over and over again.

If we can exclude stuff from e.g. bootstrap, we get a slick css which can trimmed down to the core without ever touching the sources.

Smaller css size would make websites load faster, thus saves on computer power, thus reduces CO2 emission, thus saves lives. In other words, you guys would kill people if you ignore this request. :-)

@Soviut
Copy link

Soviut commented Aug 19, 2014

@bartelsmedia That's really more of a build step using something like https://github.com/addyosmani/grunt-uncss

@bartelsmedia
Copy link

Problem is that we would need to uncss all the LESS rendered css output instead of just the LESS declarations themselve. Besides, we use Crunch on a client.

@Soviut
Copy link

Soviut commented Aug 19, 2014

@bartelsmedia Crunch is an editor, you can have Grunt or Gulp running in the background, listening for file changes and running a build process. Typically this would include compiling your LESS to CSS, then autoprefixing, then uncss, then perhaps minification with source maps.

@stale
Copy link

stale bot commented Nov 14, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 14, 2017
@stale stale bot closed this as completed Nov 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants