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

Remove redundant CSS property updates. #4261

Merged
merged 1 commit into from Jan 3, 2018

Conversation

Projects
None yet
3 participants
@schlessera
Member

schlessera commented Jan 3, 2018

Description

Some CSS properties are set twice, with the last one overwriting the previous one.

This patch optimizes the source files to only contain the needed properties once.

Resolves #4260

How Has This Been Tested?

All tests are still passing.

Types of changes

Removal of unneeded CSS properties.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
Remove redundant CSS property updates.
Some CSS properties are set twice, with the last one overwriting the previous one.

This patch optimizes the source files to only contain the needed properties once.

Resolves #4260
@aduth

aduth approved these changes Jan 3, 2018

This is great. How in the world did you find these, though? 😆 Would be fantastic if we could automate these checks with something like stylelint.

@aduth aduth merged commit 2d559e3 into WordPress:master Jan 3, 2018

2 checks passed

codecov/project 39.15% remains the same compared to eb7b0b7
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Jan 5, 2018

Member

@aduth PHPStorm will detect these. But I think the tool you've linked should detect them too.

Member

schlessera commented Jan 5, 2018

@aduth PHPStorm will detect these. But I think the tool you've linked should detect them too.

@schlessera schlessera deleted the schlessera:fix/remove-overwritten-css-properties branch Jan 5, 2018

@ntwb

This comment has been minimized.

Show comment
Hide comment
@ntwb

ntwb Jan 5, 2018

Member

stylelint didn't detect any of these dupes removed in this PR when I ran it with just the dupe rules,

It did pick up two that were not part of this PR:

blocks/editable/style.scss
 50:2  ✖  Unexpected duplicate selector ".blocks-editable__tinymce:focus", first used at line 21   no-duplicate-selectors

components/drop-zone/style.scss
 43:1  ✖  Unexpected duplicate selector ".components-drop-zone.is-dragging-over-element              no-duplicate-selectors
          .components-drop-zone__content", first used at line 23

https://github.com/WordPress/gutenberg/blob/master/blocks/editable/style.scss#L50
https://github.com/WordPress/gutenberg/blob/master/components/drop-zone/style.scss#L43

I'll take a closer look at this PR and see what we can do to have stylelint detect these dupes

Member

ntwb commented Jan 5, 2018

stylelint didn't detect any of these dupes removed in this PR when I ran it with just the dupe rules,

It did pick up two that were not part of this PR:

blocks/editable/style.scss
 50:2  ✖  Unexpected duplicate selector ".blocks-editable__tinymce:focus", first used at line 21   no-duplicate-selectors

components/drop-zone/style.scss
 43:1  ✖  Unexpected duplicate selector ".components-drop-zone.is-dragging-over-element              no-duplicate-selectors
          .components-drop-zone__content", first used at line 23

https://github.com/WordPress/gutenberg/blob/master/blocks/editable/style.scss#L50
https://github.com/WordPress/gutenberg/blob/master/components/drop-zone/style.scss#L43

I'll take a closer look at this PR and see what we can do to have stylelint detect these dupes

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