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

Import normalize.css v2 #6333

Closed
necolas opened this issue Dec 20, 2012 · 6 comments
Closed

Import normalize.css v2 #6333

necolas opened this issue Dec 20, 2012 · 6 comments
Labels
Milestone

Comments

@necolas
Copy link
Contributor

necolas commented Dec 20, 2012

You should include normalize v2 (IE8+) as-is and do away with the current custom import (spread across multiple files) of a legacy version of normalize.css. This will make it easier to update to future versions of normalize.css in bootstrap and for any issues relating to normalize.css to be passed to the upstream repo for investigation and resolution.

Any Bootstrap-specific resets/defaults that are needed can be built on top of normalize.css in a separate file.

Thanks

@mdo
Copy link
Member

mdo commented Dec 20, 2012

I'd love to do this, but have a few gripes as it stands now. Curious to hear your thoughts:

  • We use /* ... */ comments to show comments for the compiled CSS, but stripe out // ... comments. Would love to not include all the comments here in our compiled CSS. Any thoughts on changing commenting styling?
  • You use 4 spaces instead of 2 for your tabs :).

I'd still be game for including if none of these things change, but I'd still have to modify the code to address those two issues manually.

<3

@necolas
Copy link
Contributor Author

necolas commented Dec 20, 2012

Cool :)

My take on this is that one doesn't really modify library code style to meet a project-specific style. I'm not planning on changing comments (can't) or the indentation in normalize.css (maybe one day), so you're free to do so here if it fits better with what you want in Bootstrap. It's easy enough to make the reverse decision at a later date too.

@fat
Copy link
Member

fat commented Dec 20, 2012

Recess will rewrite the whitespace to be inline with the rest of bootstrap… but the comments will still be there.

Even if @necolas wanted to change the comments to // he couldn't because normalize is css not less.

I don't think it's that big of a deal though – although if i were @necolas i would change the comment style from:

/**
  *
  */

to:

/*
 *
 */

Because many css minifiers won't strip comments which begin with /*! or /**

Either way it would probably be cool to get normalize as a proper dependency in there…

@necolas
Copy link
Contributor Author

necolas commented Dec 21, 2012

The comments used to be that style, but now they use a very common format:

/**
 *
 */

Because many css minifiers won't strip comments which begin with /! or /*

Any CSS minifiers that don't strip /**-style comments need to be patched :)

@mdo
Copy link
Member

mdo commented Dec 21, 2012

Handled in #6342.

I updated the comments to use the // syntax and changed to 2 spaces instead of 4. Otherwise, the file's contents are unaltered and everything else that was in there got moved to scaffolding.less.

@mdo mdo closed this as completed Dec 21, 2012
@necolas
Copy link
Contributor Author

necolas commented Dec 21, 2012

Thanks guys :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants