maintainable-ize normalize

[m-e-h]

In it's current form the normalize.scss file is a bit of a mystery.
So is normalize being used as an un-editable drop in? It looks like it has been edited once since it was first added.

Is it something that was planned to be kept in sync with the Normalize project? If so, what version are we currently using? There are no comments or file headers to indicate this. I'm guessing it's around version 4 since the menu html element is being styled.

I say we forget about normalize (or anything else) as a drop-in and create a reset that fits and evolves with _s.

Sure, use normalize.css as a reference but there are others like sanitize.css and Bootstrap's reboot.css that have well documented solutions for reference too.

If this IS normalize.css, maybe update it and keep the header info and comments in the file.
If it's NOT normalize.css, maybe rename the file to eliminate confusion. Also folks might be more willing to make pull requests on it.

[davidakennedy]

Hi @m-e-h! Thanks for bringing this up.

When Normalize was originally added, it was done so to make updating a reset easier. Also, it was added so that we weren't maintaining our own bespoke reset. See: 6c47be9

We may have deviated from it slightly, I'd need to search through the commit history a bit more for the reasons on that.

That said, I'd be for updating Normalize. I'd rather not add our own reset since it's more code to maintain, and Normalize seems like a battle-tested solution at this point. I also wouldn't be opposed to using another solution if it's a better fit.

[m-e-h]

Normalize is good.

As for the lone modification: #895
Basically, a lot of folks were having to override one of normailize's styles.
Which is one reason why I think maybe changing the name of the file is a good idea. A third party file like that makes things awkward. "Can we change something in the file or do we override it later in our CSS?".
If you override, your coding sloppy. If the file's edited it's not normalize.css anymore or it is but it's just more difficult to update.

Maybe start with the updated normalize.css but change the file name so people feel comfortable suggesting changes to it. Normalize supports ie8+ which isn't totally compatible with _s anyway.

If we do just want to update normalize and keep it current, not having the version in the header of the file makes it very hard to maintain as a third party file.
Is there a reason the header and comments were stripped out? Is that the preferred format if and when normalize.scss is updated to the current version?

[grappler]

The issue #895 has been fixed upstream so we should be ok in updating. necolas/normalize.css@fe56763

Normalize supports ie8+ which isn't totally compatible with _s anyway.

Is there a Normalize version that only supports IE11+? I don't think having Normalize IE8+ will have a negative affect. I am not a CSS expert.

Maybe start with the updated normalize.css but change the file name so people feel comfortable suggesting changes to it.

I would suggest keeping the name. Theme developers should know how to work with normalize.css. I think a better solution would be to direct to some documentation for people starting out.

Is there a reason the header and comments were stripped out?

I could not find any reason in the history. Maybe @obenland can remember why? 😄 I think adding the version number in the style.css header would not be bad.

There is already a PR to update Normalize #1155

[m-e-h]

Good deal. Yeah ie8+ isn't a bad thing. Not really even sure why I pointed that out 😃

I think documentation, like you said, is the best thing for developers.
The reason I opened this issue was because of the investigating I had to do to figure out what version of normalize was being used and if it had been or could be modified from the original.

I'm not 100% but I think SCSS will remove single line comments starting with // .
If that's the case, maybe the only modifications we make to normailize.css when changing it to normalize.scss is to add // before each comment.
That way the style.css will stay clean of normalize comments but developers can still read them in the .scss file.
So:

// /**
//  * 1. Remove the gray background on active links in IE 10.
//  * 2. Remove gaps in links underline in iOS 8+ and Safari 8+.
// */

a {
  background-color: transparent; // /* 1 */
  -webkit-text-decoration-skip: objects; // /* 2 */
}

What do you think?
Am I looking too deep into this? 😖

[mtomas7]

Currently, _s uses a condensed version of the Normalize.css. If we want to keep the original comments, we have to decide to use original styles, not condensed. Was it done before to make scss easier to compile?

[Ismail-elkorchi]

That way the style.css will stay clean of normalize comments but developers can still read them in the .scss file.

@m-e-h, as far as I know, developers are the only people who use _s, therefore, removing the comments should be their job, as a part of their build process.
Also, I found that the style.css file has 17 comments when searching for /* (with a space after it), so normalize.css comments should be included too, if not, all the comments should be removed.
Another reason to keep normalize.css comments in style.css, is that, a lot of people customize it directly, without looking at sass files, so having comments in style.css would be very helpful for this category of people.

I suggest using the original normalize.css as it is, without any modification, as we will benefit from several advantages :

1. Less code to maintain.
2. Beginners will benefit from the comments and would not have to check both versions, to see which comment applies to which css rule.
3. As @mtomas7 stated in Update Normalize.css to v7.0.0 #1155 (comment), the normal version makes it much easier to compare with the original Normalize.css source.

[m-e-h]

@Ismail-elkorchi I agree. I'm not trying to remove comments, I'm suggesting a compromise so they can be included. I'm just assuming comments were originally removed to have a clean style.css.

I actually think comments in normalize.css are more necessary than in other CSS. And not just for beginners. There are probably many seasoned developers who would look at this and, without a comment, think it's a mistake.

pre {
  font-family: monospace, monospace;
  font-size: 1em;
}

or this

b,
strong {
  font-weight: inherit;
}

b,
strong {
  font-weight: bolder;
}

[Ismail-elkorchi]

@m-e-h we don't disagree 😃. My point was to suggest including normalize.css as it is, and without adding // before each comment, as I think that people should be able to read the comments whether they modify sass files or style.css directly, and that developers are responsible for compressing out their websites' assets.

[davidakennedy]

I'd be for displaying the comments in both Sass and main stylesheet. That may change depending on decisions around a build process, but we can figure that out later. I'd also be for just including normalize as it is in the "normal" version, so it's easier to maintain.

@mtomas7 Feel free to update your pull request.

[grappler]

@mtomas7 has kindly updated his pull request but I see that normalize uses spaces instead of tabs. The WordPress coding standards state that we should use tabs.

Do we want to make that one small change from spaces to tabs?

[crunnells]

Yes, normalize should be updated to use tabs instead of spaces.

[obenland]

The WordPress coding standards state that we should use tabs.

I'm pretty sure that doesn't apply to external libraries

[crunnells]

#1272 includes normalize.css with all of the comments, resolving this issue. It has had the spaces changed to tabs, but as @obenland mentioned, I think in the future we can include normalize.css as-is without changing the formatting.