Make WordPress Core

Opened 3 years ago

Closed 5 weeks ago

#53874 closed enhancement (fixed)

Bundled Themes: Measurement in 'px' is unnecessary

Reported by: ankitmaru's profile ankitmaru Owned by: karmatosed's profile karmatosed
Milestone: 6.6 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch
Focuses: css, coding-standards Cc:

Description

Measurement in 'px' is unnecessary in bundled themes.

Attachments (1)

measure_px_is_unnecessary_theme_bundle_css_53874.patch (10.1 KB) - added by ankitmaru 3 years ago.

Download all attachments as: .zip

Change History (18)

#1 @SergeyBiryukov
3 years ago

  • Milestone changed from Awaiting Review to 5.9
  • Summary changed from Measurement in 'px' is unnecessary in bundled themes. to Bundled Themes: Measurement in 'px' is unnecessary

#3 @netweb
3 years ago

  • Keywords commit added

This looks good, this change is also defined in the WordPress Coding Standards

https://github.com/WordPress/gutenberg/blob/trunk/packages/stylelint-config/index.js#L67

Via the stylelint rule https://stylelint.io/user-guide/rules/list/length-zero-no-unit/

... eventually we'll get stylelint linting core, and passing 🤞🏼

#5 @mukesh27
3 years ago

  • Keywords needs-refresh added

#6 @netweb
3 years ago

  • Keywords commit removed

I've removed commit from here to, running the same commands from the comments in #53866 will help cleanup the shorthand properties in themes too that touch the same lines of code:

#7 @ryelle
3 years ago

Both Twenty Twenty and Twenty Twenty-One have stylelint configs already, so these two rules could also be added to those config files: length-zero-no-unit, shorthand-property-no-redundant-values.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


3 years ago

#9 @audrasjb
3 years ago

As per today's bug scrub, let's move this ticket to Future release since it’s not ready yet, we need new stylelint configs.

#10 @audrasjb
3 years ago

  • Milestone changed from 5.9 to Future Release

#11 @sabernhardt
15 months ago

If we add the stylelint configs, the admin toolbar variable will need an exception. According to ticket:52564#comment:2, the pixel unit is necessary for addition and subtraction in calc().

This ticket was mentioned in PR #6541 on WordPress/wordpress-develop by @sabernhardt.


6 weeks ago
#12

  • Keywords needs-refresh removed
  • Adds shorthand-property-no-redundant-values to Twenty Twenty-One's stylelint rules, which updates some values in the theme's stylesheets with CSS shorthand.
  • Removes px from zero values in Twenty Ten, Twenty Eleven and Twenty Nineteen.
  • Moves box-shadow prefixed rules before the standard property in Twenty Ten.
  • Removes empty spaces at the ends of lines in Twenty Thirteen, Twenty Nineteen and Twenty Twenty (measure_px_is_unnecessary_theme_bundle_css_53874.patch included some of those changes and I found others).

Trac 53874

#13 @sabernhardt
6 weeks ago

Twenty Twenty-One already had the length-zero-no-unit rule, excluding custom properties, and I added the CSS shorthand rule.

"length-zero-no-unit": [true, {"ignore": ["custom-properties"]}],
"shorthand-property-no-redundant-values": true,

Twenty Nineteen and Twenty Twenty seem to require more than just adding the two rules to stylelint settings, so I did not add stylelint to those themes. Linting only found two values to change in Twenty Nineteen years ago, and those are fixed in the pull request.

#14 @SergeyBiryukov
6 weeks ago

  • Milestone changed from Future Release to 6.6

@mukesh27 commented on PR #6541:


5 weeks ago
#15

@SergeyBiryukov could you please take a look when you have moment?

#16 @karmatosed
5 weeks ago

I can take a look at this, so assigning to myself now.

#17 @karmatosed
5 weeks ago

  • Owner set to karmatosed
  • Resolution set to fixed
  • Status changed from new to closed

In 58178:

Bundled Themes: Removes measurement in px for zero and related fixes.

The measurement in px for zero values is not needed. It does this for Twenty Twenty-One, Twenty Ten, Twenty Eleven and Twenty Nineteen. This also adds short-hand-property-no-redudant-values to Twenty Twenty-One's stylelint rules. Included is removing box-shadow prefixed rules before the standard property in Twenty Ten. Finally, it also removes empty spaces at the ends of lines in Twenty Thirteen, Twenty Nineteen and Twenty Twenty.

Props ankitmaru, SergeyBiryukov, netweb, mukesh27, ryelle, audrasjb, sabernhardt.
Fixes #53874.

Note: See TracTickets for help on using tickets.