WordPress.org

Make WordPress Core

#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 16 months ago.
36754.3.diff (746 bytes) - added by Ipstenu 16 months ago.
built from develop
36754-toolbar.png (25.4 KB) - added by Ipstenu 16 months ago.
Toolbar change - before and after
36754-dashboard.png (31.3 KB) - added by Ipstenu 16 months ago.
Dashboard change
36754.4.diff (1.1 KB) - added by jeremyfelt 16 months ago.
37362.patch (1.4 KB) - added by ocean90 16 months ago.
36754.2.diff (1.7 KB) - added by jeremyfelt 16 months ago.

Download all attachments as: .zip

Change History (23)

@Ipstenu
16 months ago

#1 @johnjamesjacoby
16 months ago

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

#2 @ocean90
16 months 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
16 months ago

built from develop

@Ipstenu
16 months ago

Toolbar change - before and after

@Ipstenu
16 months ago

Dashboard change

#3 follow-up: @Ipstenu
16 months ago

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

#4 in reply to: ↑ 3 @johnjamesjacoby
16 months ago

Replying to Ipstenu:

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

New patch looks perfect to my eyes.

#5 @kraftbj
16 months ago

  • Keywords needs-screenshots removed

#6 @netweb
16 months ago

  • Keywords has-screenshots added

#7 @DrewAPicture
16 months ago

  • Keywords commit added

@jeremyfelt
16 months ago

#8 @jeremyfelt
16 months 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
16 months 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
16 months ago

#10 follow-up: @ocean90
16 months 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
16 months 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 (in dashicons.css), we only want to implement the multisite dashicon where it was intended (in menus and such.)

Last edited 16 months ago by johnjamesjacoby (previous) (diff)

#12 @johnjamesjacoby
16 months 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
16 months 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
16 months ago

#14 follow-up: @jeremyfelt
16 months 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
16 months ago

Replying to jeremyfelt:

Is icon16 just for back-compat?

Yep. 36754.2.diff looks good.

#16 @jeremyfelt
16 months 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.