suitcss / suit

Style tools for UI components
http://suitcss.github.io/
3.8k stars 229 forks source link

0.4.0: media-specific class naming convention #29

Closed philipwalton closed 10 years ago

philipwalton commented 11 years ago

I'm wondering if you've given any thought to formalizing a naming convention for media specific classes. I've noticed you have media specific component classes in the grid-layout module, but I don't see any mention of naming recommendations in the docs.

I imagine most of the time a media specific class would conceptually fall into the realm of utility classes (e.g., something that sets display:none or converts inline-block to block when the viewport is less than 30em), but as your grids exemplify, they may also apply to component classes.

After giving it a bit of thought I think a named media query prefix makes the most sense. The convention would be:

.[mediaName-]ComponentName { }
.u-[mediaName-]utilityName { }

If you named your media query hand to refer to anything less than 30em, then you'd get something like this:

@media (max-width: 30em) {
  .u-hand-hidden {
    display: none !important;
  }  
  .u-hand-blockList {
    display: block !important;
  }
}

Thoughts?

lukasmlady commented 11 years ago

In my experimental fork of the SUIT toolkit, I tried to address this feature. See responsive guide and an example here.

In short, the utility or component would be prefixed with a named media query like v[n]:

/* v2 breakpoint */
@media (min-width: 24em) {
  .v2-u-size2of5 {
    width: 40% !important;
  }  
  .v2-u-block {
    display: block !important;
  }
  .v2-Grid--3col > .Grid-cell {
    width: 33.3333%;
  }
}

There might be a variation like v[n]-only or v[x]-to-v[y]:

/* v2 only breakpoint */
@media (min-width: 20em) and (max-width: 23.9375em) {
  .v2-only-u-size2of5 {
    width: 40% !important;
  }  
  .v2-only-u-block {
    display: block !important;
  }
  .v2-only-Grid--3col > .Grid-cell {
    width: 33.3333%;
  }
}

/* v2 to v5 breakpoint */
@media (min-width: 24em) and (max-width: 41.9375em) {
  .v2-to-v5-u-size2of5 {
    width: 40% !important;
  }  
  .v2-to-v5-u-block {
    display: block !important;
  }
  .v2-to-v5-Grid--4col > .Grid-cell {
    width: 25%;
  }
}

As for the components as a whole, I don't think there are enough usecases.

I'm using this approach on a website and it works great so far.

necolas commented 11 years ago

Yeah, we're using this format at the moment:

.v2-u-isHidden { /* ... */ }

I'll get around to updating the documentation.

philipwalton commented 11 years ago

Perhaps this will become clearer after it's in the docs, but my first impression was that the .v[n] prefix seemed a little weird (and somewhat confusing).

When I first saw it in the grid-layouts file I honestly thought it was three different versions (like, software versions) of the grid. It was only after I started reading the CSS rules that I realized its purpose. Also, what does the v stand for? Version in the general sense? Viewport? It's a little unclear.

Secondly, my preference would be for the media query name to come after the u- prefix in utility classes. When scanning a list of classes in an HTML template, I think the most important distinction is whether it's a utility or a component. Its media type I believe is secondary, and should therefore come after the u-.

necolas commented 11 years ago

v stands for "variant". We've definitely found it easier to read all classes scoped to a Media Query by having the prefix rather than inserting it in the utility. Especially since there are already instances of utilities with 2 hyphens, e.g., u-linkComplex-target.

necolas commented 10 years ago

After thinking about this a bit more, I'm more in favour of the notation Philip suggested. The only existing utility class that uses an extra hyphen is u-linkComplex-target; it can be renamed to u-linkComplexTarget.

.u-linkComplex {}
.u-linkComplexTarget {}

Rather than names like hand, desk I'm thinking sm, md, lg:

.u-size1of2 {}

@media (min-width: 30em) {
  .u-sm-size1of2 {}
}

@media (min-width: 40em) {
  .u-md-size1of2 {}
}

@media (min-width: 50em) {
  .u-lg-size1of2 {}
}
ry5n commented 10 years ago

I had been feeling similarly dissatisfied with the v[n] prefix. On a recent project I tried out a pretty different convention, which might be interesting as food for thought:

.u-size1of2 {}

@media (min-width: 420px) {
  .u-size1of2\@md {}
}

@media (max-width: 419px) {
  .u-size1of2\@upto-md {}
}

I like the clarity of the resulting markup (but don’t love it). More importantly though, the need to escape the @ in CSS is an issue for the readability of the stylesheet.

necolas commented 10 years ago

Any feedback on this change @philipwalton and @MoOx? Have either of you been using another convention that is worth thinking about?

philipwalton commented 10 years ago

Yes, I like this change. It's actually the exact pattern I used in my last project, and I've had no issues.

And since my original suggestion, I've begun to prefer descriptors like sm, md, and lg to things like palm, hand, desk, etc., and definitely more than v[n].

+1

MoOx commented 10 years ago

I don't like to use device as name (stuff like palm, hand, desk, mobile etc). So for my concern, I am using modifiers like size-xs, size-s, size-m, size-l, size-xl. For now I am really using that as modifier so I am suffixing classes with those name.

.u-size1of2 {}

@media size-m-and-up { /* I'm using rework-breakpoint */
    .u-size1of2--size-m {}
}

Here we can see it's a bit stupid with the size usage, but I dislike sm md & lg which remind me differents things than viewport size :) Maybe view-s, view-m, view-l can do the job.

AntonTrollback commented 10 years ago

I also remember being a bit confused by the .v[n] prefixes. The sm, md, lg scale is widely used and has the ability to be extended both up and down (xs, xxs).

MoOx commented 10 years ago

Maybe a least a vp- prefix somewhere ? To know what you are talking about ? @AntonTrollback "widely used". Can you show me where ? I never saw this notation :)

AntonTrollback commented 10 years ago

@MoOx Bootstrap :)

lancejpollard commented 10 years ago

If the general naming conventions can be simplified to just use dashed-case and [verb]-[name] for states/mixins/utilities, then we can do the same for media queries with human-readable class names.

https://github.com/suitcss/suit/issues/58

.button.on-mobile
.button.on-tablet
.button.on-desktop

.button.on-small
.button.on-medium
.button.on-large

This would mean the general syntax becomes:

So then you could write clean class names in html like this:

<a class="button as-default is-disabled on-mobile">…</a>

Thoughts?

philipwalton commented 10 years ago

This reads nice, but it's not just a simplification, it's also specificity change. It would have much broader implications than just being "human-readable".

lancejpollard commented 10 years ago

@philipwalton What are some examples of how this is a specificity change?

lancejpollard commented 10 years ago

The only implications I can think of at the moment are:

Otherwise, it doesn't effect specificity at all. You can accomplish everything that's being accomplished already. Just limit the scope of the states/mixins/viewports/utilities to being appended on a component class, like suitcss is recommending in other places:

/* yay */
.button.as-primary.is-active.on-mobile {

}

/* nay, defining styles for these things without reference to a component */
.as-primary {

}

To accomplish the same thing, you would have to do just as much, but it's not as clear what's going on:

.v1-Button--primary.is-active {

}

One additional reason why this more modular approach is preferred is because it allows for composing all kinds of combinations of classes without restriction, whereas with the way it is now, you have to create a new class for every combination, which means you have to repeat a lot.

philipwalton commented 10 years ago

in some places it requires using 2 classes instead of 1, such as with .button.as-primary vs. .Button--primary

Yeah, that's exactly what I was describing. The two-class approach doubles the specificity, which means that if a particular HTML element uses two component classes, unpredictable things will happen if one of those classes uses a modifier.

It's a trade off. You could solve that by using two HTML elements instead of one, but then you're increasing verbosity in the HTML to save verbosity in the CSS.

My point was simply that your suggested change isn't just a change to the naming convention, nor is it just a matter of personal preference. It's actually a change to the entire philosophy behind the methodology.

AntonTrollback commented 10 years ago

Check out #37

lancejpollard commented 10 years ago

That article was great, thanks for pointing it out. Will think about that js-* thing some more and try it out. Haven't been needing that for personal projects b/c if everything is like angular templates with directives, you don't really ever need it. But for big teams or with legacy code I can definitely see that being helpful.

@AntonTrollback Is the issue you referenced (and the problem you mentioned you ran into) based on using the same syntax is-[name] for both modifiers and states? I definitely agree that is a problem. That is why I have proposed using two separate naming conventions for them, is-[name] for states, and as-[name] for modifiers.

By having two separate (but similar) naming conventions, you still get what @philipwalton is describing:

value in separating (via a naming convention) styles that modify components at load time verse styles that modify components at runtime via user interaction

Will play around with that js-* idea, try and get a better sense of what you're saying and get back to you. I like the idea of being able to scan the CSS to make sure no js-* appears in it.

your suggested change isn't just a change to the naming convention, nor is it just a matter of personal preference. It's actually a change to the entire philosophy behind the methodology.

@philipwalton How is it a change to the entire philosophy? I see that there is a small change by breaking that 1 .Button--primary into 2 .button.as-primary, but as long as you follow that convention, the philosophy is the same. How is it not, can you provide some code snippets or an example so I can understand where you're coming from?

The two-class approach doubles the specificity, which means that if a particular HTML element uses two component classes, unpredictable things will happen if one of those classes uses a modifier.

Isn't that an anti-pattern? I mean the whole point of creating these components is to make it so there is only one component per DOM element, right? This means you would never have a case like this:

<table class="table popup as-promoted is-open">

Instead, you would have to wrap that table in another div:

<div class="popup as-promoted is-open">
  <table class="table">
</div>

By enforcing only one component class per DOM element, modifiers and states will always be isolated.

If you're suggesting that, in fact, you can have multiple components per component DOM element, then what's stopping people from doing this?

<div class="popup accordion list table">

at which point we're back to regular old css.

AntonTrollback commented 10 years ago

@lancepollard See issue comments about specificity. It's a deal breaker, imo.

only one component per DOM element

You sometimes have to tweak component based on context

<div class="Tweet">
  <button class="Tweet-action Tweet-action--retweet Button Button--small">
</div>
lancejpollard commented 10 years ago

Ah, that's interesting. Doesn't seem ideal, wonder if there are any other ways if doing it. Have you tried any other ways? Seems like allowing multiple components like that would break the simplicity, will play around wih it though. Thanks for the example.

philipwalton commented 10 years ago

@lancepollard one component per element is a good rule of thumb, but every once in a while you have to break it. The same thing goes for context depended selectors (e.g. .Modal > .Button), they're generally a bad idea, but every once in a while you need them.

When two classes are on the same HTML element and they're setting some of the same CSS properties, there's going to be conflicts. The trick is managing the expectations for how those conflicts are resolved. The winning selector is either going to be the one that's later in the source order or the one that's more specific. I generally find source order is easier to manage since you can create your own dependency graph when compiling your CSS. Specificity, on the other hand, is impossible to trump without upping the specificity of the other selector. And that gets out of hand pretty quickly.

Note that all of my objections to your proposed naming change are in regards to the specificity increase alone. In general I like how it reads, and I want to think a bit more about it before responding more generally on the other issue.

lancejpollard commented 10 years ago

@philipwalton nice, that all makes sense. Would you (and @AntonTrollback as well) mind posting some HTML/CSS snippets from your team code? Something that shows a complex real-world example of how things get tricky. That would be super helpful for me to understand where you're coming from. It would also provide a concrete example to test out some ideas on.

lancejpollard commented 10 years ago

You guys mentioned @necolas at twitter is using this, seeing a complex example from Twitter's site would be helpful too. Looking at the profile section now, it seems they're using an older version?

view-source:https://twitter.com/twitter

<a class="media media-thumbnail twitter-timeline-link media-forward is-preview" data-url="https://pbs.twimg.com/media/BhRzSBeCUAEEbG7.png:large" data-resolved-url-large="https://pbs.twimg.com/media/BhRzSBeCUAEEbG7.png:large" href="//twitter.com/TwitterData/status/438101817398788096/photo/1/large">
  <div class=" is-preview">
    <div class="js-media-img-placeholder"
      data-img-src="https://pbs.twimg.com/media/BhRzSBeCUAEEbG7.png"
      data-img-offset="-0.0px" data-img-alt="Embedded image permalink"
      data-width="100%" style="min-height:185px">
    </div>
  </div>
</a>

@AntonTrollback is that like an example of how many different components you might attach to a single DOM element?

simonsmith commented 10 years ago

@lancepollard It's on the new Twitter profile design and sadly it's not rolled out to everyone just yet. I seemed to be upgraded to it the other week and I'm seeing Suit implemented there.

necolas commented 10 years ago

To accomplish the same thing, you would have to do just as much, but it's not as clear what's going on:

.v1-Button--primary.is-active {
}

We've never used v1- on components, and that part of the docs will be removed. Components that want to make assumptions about their width at a given media query can use a media query in the definition. If you want to be able to apply that change in a configurable manner, then it's better to use a state class.

Will think about that js-* thing some more and try it out.

That will also be going away. It's not needed/wanted in a component-based system.

By enforcing only one component class per DOM element, modifiers and states will always be isolated.

It helps. If you have a component management system to ensure that the assets for your dependencies are loaded/inlined before your component's assets, then the existing conventions have some benefits. Since the specificity of components, modifiers, and descendants are all 0,0,1,0, you can later override styles for components your component depend on without doing anything special to avoid the specificity problem that your suggestion would involve.

Looking at the profile section now, it seems they're using an older version?

That's legacy source code. We have issues in our use of SUIT, but they're primarily related to our templating system and design consistency.

simonsmith commented 10 years ago

That will also be going away. It's not needed/wanted in a component-based system.

Interesting. How will you attach JS behaviour without them? To the component class?

lancejpollard commented 10 years ago

@necolas yeah makes sense about just using media queries. glad to hear the js-* stuff is being removed too, that doesn't seem necessary for a component-based system since it could be another layer on top if desired.

necolas commented 10 years ago

How will you attach JS behaviour without them?

Check out how Flight or React work.

simonsmith commented 10 years ago

I'm aware of those libraries but I assumed Suit could be applied to normal sites as well, not just JS web apps. By that I mean ones that have a sprinkling of jQuery for example. These sites can still benefit from a modular approach but would need JS hooks.

necolas commented 10 years ago

Sure, they can make the best of it. But SUIT is intended for a component-based approach to UI development, rather than catering to every possible approach.

simonsmith commented 10 years ago

I see. I guess I hadn't seen component based as strictly related to JS apps. I can still use the JS classes though, so no bother.

necolas commented 10 years ago

You can also define components with a suitable server-side library (React is both).

necolas commented 10 years ago

Fixed in the relevant utilities.

MoOx commented 10 years ago

@necolas now maybe you should update doc accordingly https://github.com/suitcss/suit/blob/master/doc/naming-conventions.md#other

necolas commented 10 years ago

Yes, I'll get to that.

timkelty commented 10 years ago

We've never used v1- on components, and that part of the docs will be removed. Components that want to make assumptions about their width at a given media query can use a media query in the definition.

I've been using media prefixes in my work with SUIT, primarily for grids, and found them very useful. I've used a system similar to the deprecated Grid Layouts component, which uses media prefixes on a .Grid component.

I've found that system very flexible and friendly when it comes to CMS driven content and templating.

I understand why you wouldn't want to do this for every component, but it would be nice if there remained a documented way to prefix components within SUIT conventions.

timkelty commented 10 years ago

Am I the only one using a "Grid Layouts" style of grid, or see the value of being able to media prefix Components and not just utilities?

For me one of the beauties of the Grid Layout approach comes in when templating. When dealing with a dynamic listing, it's very nice to have grid classes on the "guts" never change (.Grid-cell), and the .Grid parent.

philipwalton commented 10 years ago

@timkelty yeah, actually I was just looking at this the grids in my solved by flexbox demo and sure enough I use media prefixes at the component level.

I wonder if this particular case is different. The reason I was using a media prefix on the component was because that way it could effect all grid cells at the same time. (I suspect your reasoning is similar.)

I suppose this logic could apply to any type of component that contains multiple children of the same type, e.g. a list container.

I wonder if this isn't really a component responsibility and perhaps more of a super-utility. Anyway, just thinking out loud...

timkelty commented 10 years ago

@philipwalton Targeting all the children is part of the reason, but I still think media-prefixed components are valuable in other cases as well.

Here's some examples of how I've used them recently (apart from .Grid):

.PipedList is a delimited horizontal list, .VertList is a stacked list.

<ul class="md-PipedList VertList">...

.Columnize and it's modifiers apply CSS columns to a container:

<div class="md-Columnize md-Columnize--4 lg-Columnize--6">...

I've also used prefixes on my .Container. In this case, I needed to remove the default padding, just for md:

<div class="Container md-Container--bleed">

In all these cases, the application of the classes is content and position dependent, so I wouldn't want it to be part of the component definition.

I also made a Sass mixin to facilitate this. This way I can do something like this to generate all the variants I need:

@include breakpoint-prefix(".Columnize--4") {
  @include column-count(4);
}
AntonTrollback commented 10 years ago

Nice @timkelty

timkelty commented 10 years ago

Unless I'm missing it, it appears all media query prefixing, including for utilities, has been removed from the docs. However, it does still show up here, with the syntax proposed earlier in this thread: https://github.com/suitcss/utils-size

I'm still left wanting a formalized way to prefix both Components and Utilities for media queries. Anyone else?

@necolas, Do you think this issue should be reopened?