search

Compass / compass

Compass is no longer actively maintained. Compass is a Stylesheet Authoring Environment that makes your website design simpler to implement and easier to maintain.
http://compass-style.org
Other
6.73k stars 1.18k forks source link

reset now conflicts with establish-baseline #859

Closed mirisuzanne closed 12 years ago

mirisuzanne commented 12 years ago

establish-baseline was changed in 48bfe186 to assign the base-font settings to the HTML element rather than the body element. That was a good choice overall, but it creates a conflict with the compass reset:

/* from reset */
body {
  line-height: 1;
}

/* from establish-baseline */
html {
  font-size: 14px; 
  line-height: 1.143em; 
}

The reset line-height assigned to body will override the authors line-height on html because it has higher specificity in the cascade. That's a problem.

cc/ @JohnAlbin

JohnAlbin commented 12 years ago

Well, this is why I don't like reset styles. :-) cough https://github.com/JohnAlbin/normalize.css-with-sass-or-compass cough ;-)

But the easy solution here seems to be to change the reset to use the html element as well. Since resets are supposed to be run before anything else, the cascade will take care of letting vertical-rhythm override the reset.

I've added a code comment to explain why we differ from Eric Meyer's original post.

The only odd bit left in the code is that the mixin that adds the line-height: 1 is still called @body-reset even though we're using to reset the html element. But I personally doubt this is used directly by anyone, so leaving it with the same name will be confusing to very few people. And those people will already be using the mixin and probably don't want the mixin to break just because we wanted to rename the mixin to match how its applied in @global-reset.

mirisuzanne commented 12 years ago

:) Yes well, there's more debate to be had about normalizing, but in the meantime we need the reset to work.

I like the solution. Have you tested that resetting on the html accomplishes the same thing? I assume it would, but I'd just like to be sure.

I think our usual approach to the name issue would be to rename it, but leave the old one in place as well - probably a simple wrapper that calls the new mixin. Or possibly leave it completely intact, and just don't call it in the overall reset. Thoughts?

scottdavis commented 12 years ago

@ericam if this is going with a new version number i dont see any reason why we would have to take that in to account. As long as the change is transparent to the user and documented in the changelog (some people read them) it shouldn't be an issue. I want to avoid having stray mixins that are simple wrappers for regressions. If we are going to move forward we should.

mirisuzanne commented 12 years ago

Fine with me. I'll merge it in.