Skip to content
This repository was archived by the owner on Sep 5, 2025. It is now read-only.

Comments

Format SCSS#1168

Closed
m-e-h wants to merge 1 commit intoAutomattic:masterfrom
m-e-h:format-scss
Closed

Format SCSS#1168
m-e-h wants to merge 1 commit intoAutomattic:masterfrom
m-e-h:format-scss

Conversation

@m-e-h
Copy link

@m-e-h m-e-h commented Jul 7, 2017

Changes proposed in this Pull Request:

Lint and properly format SCSS so it'll produce a properly formatted style.css

Related issue(s):

#1149
#1159
#932
#893

@m-e-h
Copy link
Author

m-e-h commented Jul 8, 2017

Oops. Looks like I need to squash a few commits here. Reckon I should add the compiled style.css too...

@m-e-h
Copy link
Author

m-e-h commented Jul 10, 2017

The updated style.css has a couple minor quirks due to node-sass and/or WP CSS coding standards.

  1. empty selector blocks for the menu section only appear in the SASS files.
  2. single line comments on the same line as a parameter have been changed from /* */ to //, so they only show in the SASS files, as well. This is because during compilation the /* */ was being moved to it's own line.

If automating the style.css generation through SASS compilation is the goal, I don't see many ways around making some adjustments like this.

I did still have to make one manual fix to the style.css.
I think it was node-sass that didn't seem to like this piece of code.

.comment-navigation,
.posts-navigation,
.post-navigation {

	.site-main & {
		margin: 0 0 1.5em;
		overflow: hidden;
	}
}

It produced this in the CSS

.site-main .comment-navigation,
.site-main
.posts-navigation,
.site-main 
.post-navigation {
	margin: 0 0 1.5em;
	overflow: hidden;
}

The SCSS will probably need adjusted here but I thought that could be done after this commit?
Please let me know any thoughts on these or anything else that doesn't look right.

@m-e-h
Copy link
Author

m-e-h commented Jul 10, 2017

Just noticed this has some overlap with this #1153 and this #1170

@grappler
Copy link
Contributor

Thank you for the PR.

As #1149 has started to question the need for Sass we will need to wait to merge this till we have a resolution.

As we are planning to update Normalize in #1155 the pull request will need to be refresh afterwards.

@Ismail-elkorchi
Copy link
Contributor

The formatting issues of SASS and CSS were resolved in #1391

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants