WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#19122 closed defect (bug) (fixed)

Always show "My Sites", sometimes hide "Site Name" menu

Reported by: duck_ Owned by: duck_
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.3
Component: General Keywords: has-patch commit
Focuses: Cc:

Description

Non-super-admin who is member of only one site visits a site they're not associated with.

  1. They see the "Site Name" menu and it's linked to the dashboard of the current site which they don't have access to
  2. No "My Sites" menu for quick access to dashboard of the user's site

Point 1. can be fixed by hiding the "Site Name" site when the current blog ID isn't in the user's blogs array and they're not a super-admin.

Point 2. proposed fixed is to always show the "My Sites" menu.

Also stopped the removal of the current site from "My Sites" to fix issue of empty "My Sites" drop down. This could probably also be fixed by some more complicated logic earlier in the function, but the 'now it's here, now it isn't' nature seems a little strange to me.

Related: #18197, #18990 (proposed fix would invalidate the latter)

Attachments (3)

19122.diff (1.5 KB) - added by duck_ 4 years ago.
19122.2.diff (2.9 KB) - added by duck_ 4 years ago.
19122.3.diff (1.6 KB) - added by duck_ 4 years ago.

Download all attachments as: .zip

Change History (14)

@duck_4 years ago

comment:1 @duck_4 years ago

  • Keywords ux-feedback added

comment:2 follow-up: @nacin4 years ago

On WP.com, the name still shows, but it links to the public site. This is helpful since someone may use it to grab a shortlink.

Wait, where did shortlinks go?

comment:3 @nacin4 years ago

Thoughts:

We should always show My Sites (if they have a site, or are a super admin).

We should always show the Site Name, and if they're not a member, it takes them to home_url(). It's not elegant, but it's probably better to be inconsistent in where things link rather than whether it shows.

Also, having "My Sites" on the left may be good for hierarchy, but I keep thinking it should be on the right (it is user-specific) or in the profile menu. And "Network Admin" feels more hidden than "Screen Options" did.

comment:4 in reply to: ↑ 2 @nacin4 years ago

Replying to nacin:

On WP.com, the name still shows, but it links to the public site. This is helpful since someone may use it to grab a shortlink.

Wait, where did shortlinks go?

Cancel that, I had default permalinks on, so the shortlink API wasn't returning anything.

comment:5 @duck_4 years ago

19122.2.diff always shows the Site Name, but links to home_url() and doesn't add the default submenu items if not a member, and always shows My Sites if they have a site or they're a super admin.

@duck_4 years ago

comment:6 @jane4 years ago

  1. current site should not be removed from my sites if it is a site they have a role on
  1. site name menu should not show if no role on that site
  1. yes, my sites should always be there if they have sites on the network
  1. i would prefer to keep my sites on the left

@duck_4 years ago

comment:7 @duck_4 years ago

Following Jane's comment.

19122.3.diff refreshes the original patch with the following changes:

  • Use API, is_user_member_of_blog(), rather than plain array_key_exists(). See #19160 for why $blog_id has to be specified in the patch.
  • Don't show My Sites when the user has 0 sites and isn't a super admin.

comment:8 @nacin4 years ago

Can we split line 214 into two different bail conditions? The $a || $b && $c can be tough to read.

comment:9 @nacin4 years ago

  • Keywords commit added; 2nd-opinion ux-feedback removed

comment:10 @duck_4 years ago

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

In [19184]:

Hide Site Name menu if the user isn't a member and isn't a super admin. Always show My Sites if the user has a site, and don't remove the current site from the list. Fixes #19122.

comment:11 @duck_4 years ago

In [19265]:

Default arguments for is_user_member_of_blog() can now be used. See #19122, #19160.

Note: See TracTickets for help on using tickets.