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

Use the stylelint-config-wordpress\scss stylelint config #17790

Closed
wants to merge 1 commit into from

Conversation

ntwb
Copy link
Member

@ntwb ntwb commented Oct 6, 2019

Description

Via @gziolo in #17060 (comment)

The current stylelint config uses CSS rather than the SCSS config included in stylelint-config-wordpress

The stylelint-config-wordpress\scss config extends stylelint-config-wordpress so the changes will allow for improved linting of SCSS files.

How has this been tested?

  • Checkout this PR locally
  • Run npm install
  • Run npm run lint-css

Screenshots

Types of changes

Build tool change

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

Results

The following are the results of running npm run lint-css and the SCSS files should be updated also as part of this PR.

assets/stylesheets/_mixins.scss
 367:3  ✖  Expected single space after "}" of @if statement     scss/at-if-closing-brace-space-after    
 367:3  ✖  Unexpected newline after "}" of @if statement        scss/at-if-closing-brace-newline-after  
 369:2  ✖  Unexpected empty line before @else                   scss/at-else-empty-line-before          
 373:3  ✖  Expected single space after "}" of @else statement   scss/at-else-closing-brace-space-after  
 373:3  ✖  Unexpected newline after "}" of @else statement      scss/at-else-closing-brace-newline-after
 375:2  ✖  Unexpected empty line before @else                   scss/at-else-empty-line-before          

packages/block-library/src/classic/editor.scss
 117:3  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/block-library/src/code/editor.scss
 42:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/block-library/src/gallery/style.scss
 85:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector
 86:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/block-library/src/navigation-menu-item/editor.scss
 63:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector
 67:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/block-library/src/pullquote/editor.scss
  4:3  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector
 11:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector
 21:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/components/src/button-group/style.scss
 8:3  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/components/src/circular-option-picker/style.scss
 30:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector
 69:3  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/components/src/toolbar/style.scss
 10:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector
 20:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/components/src/toolbar-button/style.scss
 27:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/block-editor/src/components/block-navigation/style.scss
 47:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/edit-post/src/components/visual-editor/style.scss
 5:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

@ntwb ntwb added [Type] Build Tooling Issues or PRs related to build tooling [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. labels Oct 6, 2019
@gziolo
Copy link
Member

gziolo commented Oct 7, 2019

The following are the results of running npm run lint-css and the SCSS files should be updated also as part of this PR.

Let's do it 😄

@ntwb ntwb added the Needs Design Feedback Needs general design feedback. label Oct 7, 2019
@ntwb
Copy link
Member Author

ntwb commented Oct 7, 2019

SCSS is not my strong suit, so I'm not sure what's required to fix these issues:

packages/block-library/src/classic/editor.scss
 117:3  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

Maybe this is something you could look at and help with please @jasmussen ?

@jasmussen
Copy link
Contributor

Thanks for the ping.

If I'm not mistaken, this is the SCSS in question:

	/* Image captions */
	dl.wp-caption {
		margin: 0; /* dl browser reset */
		max-width: 100%;

		a,
		img {
			display: block;
		}

		&,
		& * {
			-webkit-user-drag: none;
		}

		.wp-caption-dd {
			padding-top: 0.5em;
			margin: 0; /* browser dd reset */
		}
	}

I totally see why it was written this way, but if it makes the linter unhappy, I believe it can be written this way too:

	/* Image captions */
	dl.wp-caption {
		margin: 0; /* dl browser reset */
		max-width: 100%;
		-webkit-user-drag: none;

		* {
			-webkit-user-drag: none;
		}

		a,
		img {
			display: block;
		}

		.wp-caption-dd {
			padding-top: 0.5em;
			margin: 0; /* browser dd reset */
		}
	}

@gziolo gziolo added the [Status] In Progress Tracking issues with work in progress label Oct 10, 2019
@ntwb
Copy link
Member Author

ntwb commented May 31, 2020

Related: #22777

@ntwb
Copy link
Member Author

ntwb commented May 31, 2020

@gziolo What's the best way to update this branch?

@gziolo
Copy link
Member

gziolo commented Jun 1, 2020

@ntwb, do you want to land #22777 first before you rebase this one?

@paaljoachim
Copy link
Contributor

Can we get a status update on where this PR is at?
What are the next steps?

@paaljoachim
Copy link
Contributor

paaljoachim commented Dec 8, 2020

I do not think this PR needs design feedback. It seems more like that it needs technical feedback.
So I am removing the needs design feedback label from it. If I am mistaken then please add it back in again.

@paaljoachim paaljoachim removed the Needs Design Feedback Needs general design feedback. label Dec 8, 2020
@gziolo
Copy link
Member

gziolo commented Jan 3, 2021

It should use @wordpress/stylelint-config with #27810 landed.

@gziolo gziolo added [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. and removed [Status] In Progress Tracking issues with work in progress labels Jan 3, 2021
@ntwb ntwb closed this Jan 4, 2021
@gziolo gziolo deleted the fix/stylelint-scss branch January 4, 2021 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants