WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#36754 closed enhancement (fixed)

Apply new Dashicon to Multisite

Reported by: Ipstenu Owned by: jeremyfelt
Milestone: 4.6 Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch has-screenshots
Focuses: ui, multisite Cc:

Description

Back in 4.3 (see #30902) we added a new icon for Multisite.

We've never added it to WordPress core.

This patch does just that, replacing the key with the houses.

Attachments (7)

36754.diff (1.4 KB) - added by Ipstenu 2 years ago.
36754.3.diff (746 bytes) - added by Ipstenu 2 years ago.
built from develop
36754-toolbar.png (25.4 KB) - added by Ipstenu 2 years ago.
Toolbar change - before and after
36754-dashboard.png (31.3 KB) - added by Ipstenu 2 years ago.
Dashboard change
36754.4.diff (1.1 KB) - added by jeremyfelt 2 years ago.
37362.patch (1.4 KB) - added by ocean90 2 years ago.
36754.2.diff (1.7 KB) - added by jeremyfelt 2 years ago.

Download all attachments as: .zip

Change History (23)

@Ipstenu
2 years ago

#1 @johnjamesjacoby
2 years ago

Would ya look at that. I'm +1 on this.

#2 @ocean90
2 years ago

  • Focuses ui added
  • Keywords has-patch needs-screenshots added
  • Milestone changed from Awaiting Review to 4.6
  • Version trunk deleted

@Ipstenu You should use develop.svn.wordpress.org because you don't have to patch RTL files. These are generated by our build tools.

@Ipstenu
2 years ago

built from develop

@Ipstenu
2 years ago

Toolbar change - before and after

@Ipstenu
2 years ago

Dashboard change

#3 follow-up: @Ipstenu
2 years ago

Added images and (theoretically) the patch from the right place.

#4 in reply to: ↑ 3 @johnjamesjacoby
2 years ago

Replying to Ipstenu:

Added images and (theoretically) the patch from the right place.

New patch looks perfect to my eyes.

#5 @kraftbj
2 years ago

  • Keywords needs-screenshots removed

#6 @netweb
2 years ago

  • Keywords has-screenshots added

#7 @DrewAPicture
2 years ago

  • Keywords commit added

@jeremyfelt
2 years ago

#8 @jeremyfelt
2 years ago

36754.4.diff catches .dashicons-admin-network:before as well, which is what was displaying the sidebar icon for me. I've lost track of where .icon16.icon-site is applied, but changing it there makes sense.

#9 @jeremyfelt
2 years ago

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

In 37362:

Multisite: Use the admin-multisite Dashicon in the admin menu and bar

This icon was added to Dashicons in 4.3, but not applied to anything in our CSS.

Props Ipstenu.
Fixes #36754.

@ocean90
2 years ago

#10 follow-up: @ocean90
2 years ago

  • Keywords commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

dashicons.css is an external-ish file and shouldn't be changed in core without changing it upstream too.

The class for the new icon is .dashicons-admin-multisite and should be used for the menu instead, see 37362.patch.

#11 in reply to: ↑ 10 @johnjamesjacoby
2 years ago

Replying to ocean90:

dashicons.css is an external-ish file and shouldn't be changed in core without changing it upstream too.

The class for the new icon is .dashicons-admin-multisite and should be used for the menu instead, see 37362.patch.

Ugh. I totally forgot about that.

Your patch seems incorrect, unless I'm reading it wrong. We do not want to change the network dashicon, we only want to implement the multisite dashicon where it was intended.

Version 1, edited 2 years ago by johnjamesjacoby (previous) (next) (diff)

#12 @johnjamesjacoby
2 years ago

Nevermind; I am reading it wrong.

@jeremyfelt slipped in a new patch that changed the network icon. That should get reverted in favor of a portion of @Ipstenu's patch upstream to Github, and @ocean90's patch will rebase core for now, too.

#13 @jeremyfelt
2 years ago

In 37364:

Revert [37362], which incorrectly changed Dashicons CSS

dashicons.css is an external-ish library and the change there is incorrect.

See #36754.

@jeremyfelt
2 years ago

#14 follow-up: @jeremyfelt
2 years ago

Ok. Reverted to restart as an entire changeset. :)

36754.2.diff applies the menu change, the #wpadminbar #wp-admin-bar-site-name > .ab-item:before change, and the .icon16.icon-site:before change.

Is icon16 just for back-compat?

#15 in reply to: ↑ 14 @ocean90
2 years ago

Replying to jeremyfelt:

Is icon16 just for back-compat?

Yep. 36754.2.diff looks good.

#16 @jeremyfelt
2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 37365:

Multisite: Use the admin-multisite Dashicon for sites

This icon was added to Dashicons in 4.3, but not applied to anything in our CSS.

Uses the more appropriate dashicons-admin-multisite when displaying the sites menu item.

Props Ipstenu, ocean90.
Fixes #36754.

Note: See TracTickets for help on using tickets.