Opened 4 years ago
Closed 4 years ago
#52148 closed enhancement (fixed)
CSS Optimization: Use shorthand 'border' value in wp-admin/css/admin-menu.css
Reported by: | ankitmaru | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 5.7 | Priority: | normal |
Severity: | normal | Version: | 5.7 |
Component: | Administration | Keywords: | has-patch commit |
Focuses: | ui, css, coding-standards | Cc: |
Description
Properties can be replaced with 'border' shorthand.
Attachments (13)
Change History (40)
#3
@
4 years ago
- Summary changed from CSS Optimization to CSS Optimization: Use shorthand 'border' value in wp-admin/css/admin-menu.css
#6
@
4 years ago
Hum… looks like it's legitimate in this case, though… because there is a different border for each side…
#7
@
4 years ago
Besides jquery-ui-dialog.css, I found one additional instance that can combine all three properties into one line:
wp-includes/css/editor.css
#wp_editimgbtn, #wp_delimgbtn, #wp_editgallery, #wp_delgallery { background-color: #eee; margin: 2px; padding: 2px; border: 1px solid #999; border-radius: 3px; }
Border styles like .contextual-help-tabs li
(comment:5) could combine to two lines, if desired.
#8
in reply to:
↑ 5
@
4 years ago
I have added few more patches as well. Checked whole css/ folder for the same improvements.
Replying to audrasjb:
Hi, thanks for the ticket and patch,
I think it would be nice to quickly check other occurence of this.
For example, in wp-admin/css/common.css:
.contextual-help-tabs li { margin-bottom: 0; list-style-type: none; border-style: solid; border-width: 0 0 0 2px; border-color: transparent; }
#10
@
4 years ago
- Keywords commit removed
@ankitmaru it would be easier for committers to merge all those patch in one unique patch, if possible :)
#12
@
4 years ago
Thanks for the quick update @ankitmaru you rock!
One small change on my side:
- the
/* equal height column trick */
comment should be kept (incommon.css
file). As this is a small CSS "hack", it's better to keep it documented ;-)
#15
@
4 years ago
- Keywords commit added
@ankitmaru there’s not need to be sorry, thank you for helping this patch to move forward!
The patch looks good to me. Marking this for commit
.
#17
@
4 years ago
- Keywords commit removed
I'm really sorry @ankitmaru but I have few more requests:
- in
common.css
replacepadding: 10px 26px 99999px 26px;
withpadding: 10px 26px 99999px;
/ andpadding: 16px 16px 99999px 16px;
withpadding: 16px 16px 99999px;
- In
customize-controls.css
replacepadding: 0px 0px 7px 0px;
withpadding: 0 0 7px;
- In
install.css
replacepadding: 0px 0px 7px 0px;
withpadding: 0 0 7px;
- In
nav-menus.css
replacemargin: 0px 0px 5px 0px;
withmargin: 0 0 5px;
See MDN Docs for further informations:
https://developer.mozilla.org/en-US/docs/Web/CSS/padding#syntax
The padding [it also applies for margin] property may be specified using one, two, three, or four values.
- When one value is specified, it applies the same padding to all four sides.
- When two values are specified, the first padding applies to the top and bottom, the second to the left and right.
- When three values are specified, the first padding applies to the top, the second to the right and left, the third to the bottom.
- When four values are specified, the paddings apply to the top, right, bottom, and left in that order (clockwise).
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#20
@
4 years ago
- Keywords needs-refresh added
I review the patches for the ticket and found the mention issues.
customize-controls.css_52148.patch
This padding: 0px 0px 7px 0px;
should replace with padding: 0px 0px 7px;
class-wp-comments-list-table.php_52148.patch
As per our docs standard return
should be at last not at first in the documentation.
This padding: 0px 0px 7px 0px;
should replace with padding: 0px 0px 7px;
This margin: 0px 0px 5px 0px;
should replace with margin: 0px 0px 5px;
#21
follow-up:
↓ 22
@
4 years ago
@ankitmaru Thanks for combining the patches and updating it in css_updated_52148.patch.
Please make two more changes:
- For the second change in customize-controls.css, remove the units with a zero value:
margin: 0 0 0 10px;
(see coding standards about zero value)
- Combine the 3 border properties in wp-includes/css/editor.css (comment:7)
If you don't have time to update the patch today, I could do that.
#22
in reply to:
↑ 21
@
4 years ago
I have fixed the point no. 2, Can you please share more info about point no. 1?
Replying to sabernhardt:
@ankitmaru Thanks for combining the patches and updating it in css_updated_52148.patch.
Please make two more changes:
- For the second change in customize-controls.css, remove the units with a zero value:
margin: 0 0 0 10px;(see coding standards about zero value)
- Combine the 3 border properties in wp-includes/css/editor.css (comment:7)
If you don't have time to update the patch today, I could do that.
#23
@
4 years ago
The styles for #customize-theme-controls .add-new-menu-item
do not need the px
units for the three zero values, because the result is the same no matter what the unit is.
#customize-theme-controls .add-new-widget, #customize-theme-controls .add-new-menu-item { cursor: pointer; float: right; margin: 0 0 0 10px; transition: all 0.2s; -webkit-user-select: none; -ms-user-select: none; user-select: none; outline: none; }
(You already did similarly with the body.cheatin h1
padding earlier in that file.)
padding: 0 0 7px;
Also, I don't think the class-wp-comments-list-table.php change belongs on this ticket. After putting the @global int $post_id
first among the global variables, you could upload a patch for that file to ticket:51800.
@
4 years ago
Refreshed to (a) change 0px to 0 and (b) remove class-wp-comments-list-table.php changes
#24
@
4 years ago
- Keywords commit added; needs-refresh removed
Today is 5.7 Beta 1. I refreshed the patch per @sabernhardt last comment.
Marking this as a commit
candidate.
Hi, thanks for the ticket and patch,
I think it would be nice to quickly check other occurence of this.
For example, in wp-admin/css/common.css: