Make WordPress Core

Opened 6 years ago

Closed 2 weeks ago

Last modified 7 days ago

#43633 closed enhancement (fixed)

Duplicate links to edit profile dropdown in toolbar

Reported by: danieltj's profile danieltj Owned by: joedolson's profile joedolson
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: Toolbar Keywords: has-patch commit
Focuses: ui Cc:

Description

Within the mini user profile drop down in the toolbar (to the right), there are two links which direct the user to the edit profile page. Not only is this confusing, it's a poor user experience as some users may wish to have a link to their author archive instead to see what they've written.

I propose this is changed so there is a link to author archives and the edit profile page.

Attachments (13)

user-profile-dropdown.png (24.7 KB) - added by danieltj 6 years ago.
Example of the mini user profile drop down
43633.diff (579 bytes) - added by danieltj 6 years ago.
Replaces edit profile with view posts
43633.2.diff (926 bytes) - added by danieltj 6 years ago.
Refreshed the patch
43633.3.diff (1.2 KB) - added by sabernhardt 3 years ago.
combining duplicate profile links within dropdown menu
combined-profile-link.png (14.4 KB) - added by sabernhardt 3 years ago.
43633.4.diff (995 bytes) - added by sabernhardt 20 months ago.
43633.3 without adding aria-hidden
no-role-WP6.4.png (8.3 KB) - added by sabernhardt 3 weeks ago.
profile menu for user without a role in WordPress 6.4
no-role-WP6.5b3.png (8.6 KB) - added by sabernhardt 3 weeks ago.
in 6.5 beta, the Edit Profile text is added for users without a role
profile-menu-700px-WP6.4.png (10.8 KB) - added by sabernhardt 3 weeks ago.
profile menu below the 782-pixel breakpoint in WordPress 6.4
profile-menu-700px-WP6.5b3.png (10.8 KB) - added by sabernhardt 3 weeks ago.
in WordPress 6.5 beta, smaller screens do not increase the font size of the Edit Profile text, and it is closer to Log Out than the rest of the link text
make-core-profile.png (21.0 KB) - added by sabernhardt 3 weeks ago.
combined link has extra space between lines on Make Core
43633.5.diff (2.6 KB) - added by sabernhardt 3 weeks ago.
restores ( false !== $profile_url ) check, adds display-name class, adds zero tabindex to profile menu without a link, removes grid row-gap, adds another space after 'meta', capitalizes the ARIA acronym, adds a translator comment
43633.6.diff (3.1 KB) - added by joedolson 2 weeks ago.
Changes item heights to be measured in rem instead of px so menu items are still readable when font size is increased.

Download all attachments as: .zip

Change History (39)

@danieltj
6 years ago

Example of the mini user profile drop down

@danieltj
6 years ago

Replaces edit profile with view posts

#1 @danieltj
6 years ago

  • Keywords has-patch needs-testing added

Added the 43633.diff patch which changes the link from an edit profile link to a view posts link.

#3 follow-up: @swissspidy
6 years ago

You'd need to change the false !== $profile_url condition there as well.

#4 in reply to: ↑ 3 ; follow-up: @danieltj
6 years ago

  • Keywords needs-refresh added; needs-testing removed

Replying to swissspidy:

You'd need to change the false !== $profile_url condition there as well.

Thanks for the heads up, I think it's worth refreshing the patch anyway to change the name and username link to the archive page rather than removing the edit profile link as that makes more sense to leave as it is so there is a clear way to edit the profile.

#5 in reply to: ↑ 4 @netweb
6 years ago

Replying to danieltj:

Replying to swissspidy:

You'd need to change the false !== $profile_url condition there as well.

Thanks for the heads up, I think it's worth refreshing the patch anyway to change the name and username link to the archive page rather than removing the edit profile link as that makes more sense to leave as it is so there is a clear way to edit the profile.

I'd much rather see the first link become a "view profile" link rather thean the current "edit profile" link:

e.g. https://profiles.wordpress.org/danieltj/profile/

The second link as noted above should remain as is

e.g. https://profiles.wordpress.org/danieltj/profile/edit

Per the above examples, most users of a bbPress site will not have any posts so linking to a users posts, or post archive when they have no posts is not what we want to do for sites such as those that use bbPress.

@danieltj
6 years ago

Refreshed the patch

#6 @danieltj
6 years ago

  • Keywords needs-testing added; needs-refresh removed

I've refreshed the patch in 43633.2.diff and added a filter so that plugins and site owners can change the URL if they wish. I think this is the best course of action as it opens up the author archive but lets people change it to whatever they like by filtering it. Essentially just letting people choose where they want this additional link to go.

Can I get tests and thoughts on this please?

#7 follow-up: @netweb
6 years ago

I stand by what I wrote in comment:5, the primary purpose of the wp_admin_bar_my_account_menu() function is to add "My Account" items to the admin bar which are the user profile links, first the user profile #L291, and then, if it is not multisite #L266 the "Edit My Profile" link is added.

43633.2.diff changes multisite behaviour by changing the first link to link to a users post archive rather than the current link to the dashboard #L267, this is currently used I expect due to all of the multisite blog/site/network juggling complexities that are required to retrieve any user related content in multisite environments.

Also adding a filter as proposed in 43633.2.diff puts the onus on developers to opt-out of a what is a breaking change being introduced into WordPress and these types of change should be avoided.

#8 in reply to: ↑ 7 @danieltj
6 years ago

  • Keywords 2nd-opinion added

Replying to netweb:

I stand by what I wrote in comment:5, the primary purpose of the wp_admin_bar_my_account_menu() function is to add "My Account" items to the admin bar which are the user profile links, first the user profile #L291, and then, if it is not multisite #L266 the "Edit My Profile" link is added.

43633.2.diff changes multisite behaviour by changing the first link to link to a users post archive rather than the current link to the dashboard #L267, this is currently used I expect due to all of the multisite blog/site/network juggling complexities that are required to retrieve any user related content in multisite environments.

Also adding a filter as proposed in 43633.2.diff puts the onus on developers to opt-out of a what is a breaking change being introduced into WordPress and these types of change should be avoided.

How do we fix the issue of having two links to the same page though? As it stands, single-site installs have two links to the same place which isn't useful and is confusing. In 43633.2.diff, the ids are kept the same to ensure additional links are still added correctly to the toolbar, the link location has simply changed.

There is another way to the dashboard on multisites anyway on the left hand side, so access to these account pages isn't lost and still very accessible. The aim here is to change the fact we have two links going to the same page right next to each other. A much better use of toolbar UI would be to provide a link to somewhere that is hard to find.

Adding 2nd-opinion to get more thoughts on this.

@sabernhardt
3 years ago

combining duplicate profile links within dropdown menu

#9 @sabernhardt
3 years ago

Adding a link to the author archive page likely is more appropriate with a plugin.

However, combining the two links seems possible with something like 43633.3.diff.

  1. "Edit profile" is included within the 'user-info' link, with the display-name class to keep the same styling. That span also has an edit-profile-link class for plugin authors who might wish to change those styles.
  2. I don't think the username adds any value for screen reader users, so it has aria-hidden (if I'm wrong, that attribute can be removed).
Last edited 3 years ago by sabernhardt (previous) (diff)

@sabernhardt
20 months ago

43633.3 without adding aria-hidden

This ticket was mentioned in Slack in #accessibility by sabernhardt. View the logs.


5 weeks ago

#11 @joedolson
5 weeks ago

#60489 was marked as a duplicate.

#12 @dionysous
5 weeks ago

I support the desired changes and find this topic very important because I think that the current linking is inadequate and far too redudant.

I'm interested in understanding how the new design here, particularly the View Profile link, is intended to appear.

For example, on wordpress.org, navigation to your own profile is made enormously difficult by the fact that you have 3x Edit Profiles and 0x View Profiles links. The drop-down menu on wordpress.org, has been customized using a MU plugin, resulting in a different appearance. Therefore there is an issue on GitHub: https://github.com/WordPress/wporg-mu-plugins/issues/268#issuecomment-1938290193

PS: As for the current design suggestion for wordpress.org, here's a visual representation:
https://i.imgur.com/p1SHeyC.png

The added value here would be that users can distinguish their accounts by their unique user names, but also see their display names. The design would not be changed too much either.

Last edited 5 weeks ago by dionysous (previous) (diff)

This ticket was mentioned in Slack in #design by dionysous. View the logs.


5 weeks ago

#14 @joedolson
3 weeks ago

  • Keywords needs-testing 2nd-opinion removed
  • Milestone changed from Awaiting Review to 6.5

Adding this to 6.5 to record that I'm committing essentially the same change as in @sabernhardt's 43633.4.diff as part of the fix for #34668.

#15 @joedolson
3 weeks ago

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

In 57708:

Toolbar: Accessibility: Keyboard navigation for screen readers.

Change the admin toolbar to have role="menu" and support opening for screen readers. Remove screen reader only log out link and collapse duplicate profile links into one link. This is an imperfect solution to a complex problem in the adminbar, but the lack of screen reader access to submenus is a major accessibility problem, and this fix provides access, even if the mechanism is imperfect.

Screen reader log out added in [21452].

Props abletec, Cheffheid, sabernhardt, alexstine, joedolson, afercia, sparklingrobots, danieltj, swissspidy, netweb, dionysous.
Fixes #34668, #43633.

@sabernhardt
3 weeks ago

profile menu for user without a role in WordPress 6.4

@sabernhardt
3 weeks ago

in 6.5 beta, the Edit Profile text is added for users without a role

@sabernhardt
3 weeks ago

profile menu below the 782-pixel breakpoint in WordPress 6.4

@sabernhardt
3 weeks ago

in WordPress 6.5 beta, smaller screens do not increase the font size of the Edit Profile text, and it is closer to Log Out than the rest of the link text

@sabernhardt
3 weeks ago

combined link has extra space between lines on Make Core

@sabernhardt
3 weeks ago

restores ( false !== $profile_url ) check, adds display-name class, adds zero tabindex to profile menu without a link, removes grid row-gap, adds another space after 'meta', capitalizes the ARIA acronym, adds a translator comment

#16 @sabernhardt
3 weeks ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I added a patch to fix some things, but we could consider reverting this part of [57708] instead and revisiting the duplicate links in another release cycle.

  • The separate "Edit Profile" link needed an if ( false !== $profile_url ) condition for users without a role in [34122], and it should continue to check that condition for the text.
  • Also for users without a role, the profile menu div would need the zero tabindex to be focusable (more related to #34668). The parameter checks for an integer, and I chose an empty string for the usual case of no tabindex. Another possibility is giving any ( $is_parent && ! $has_link ) node the zero tabindex, but there may have been a reason not to do that when the parameter was added.
  • The 12-pixel row-gap added to the combined link is too much space on Make Core and likely in other conditions too. I simply removed it in the patch for now. Editing the height, line-height and/or font-size could be better.
  • 43633.4.diff specifically had the display-name class before the new class for the Edit Profile span. This was an easy way to match its styles to the display name span without editing the color palettes. In addition to the color, smaller screens need the adjustments to font-size, line-height and height.
  • Additional edits in the patch include an extra space after 'meta' to align the array values, capitalizing the ARIA acronym, and adding a translator comment. These changes could be made if reverting is the better option.

The profile menu also does not show the image/icon on smaller screens when the site enables avatars and the user has no role, but that bug is older so I did not add absolute positioning for the image within this patch.

Last edited 3 weeks ago by sabernhardt (previous) (diff)

#17 @joedolson
2 weeks ago

I mostly committed that to get feedback, honestly. Pretty OK with reverting it, although I think minor visual issues are better than the duplicated link, overall.

Had not considered users without a role, however.

#18 @huzaifaalmesbah
2 weeks ago

Looks Good 43633.5.diff I can see that the solution has been improved with better design and issue resolution.

@joedolson
2 weeks ago

Changes item heights to be measured in rem instead of px so menu items are still readable when font size is increased.

#19 @joedolson
2 weeks ago

Actually making the adminbar functional at 200% is a large problem, but this at least makes the dropdowns readable, even if the top level items aren't. That simply stood out more now that the profile links are grouped inside one item.

#21 @joedolson
2 weeks ago

  • Keywords commit added

I think this is good. It improves on a prior change, includes some important changes to code quality and some changes to design. Given the choice between improvement and reverting, I'm in favor of improvement.

#22 @joedolson
2 weeks ago

Noting for the record that I mathed wrong in the patch, and changed the item height to 1.875rem instead of 1.625rem; but fixing that in commit.

#23 @joedolson
2 weeks ago

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

In 57765:

Toolbar: Polish design and code combining duplicate profile links.

Fixes some design changes and improves quality of comments and code styles following previous changes. Follow-up to [57708].

Props sabernhardt, huzaifaalmesbah, joedolson.
Fixes #43633. See #34668.

@joedolson commented on PR #6222:


2 weeks ago
#24

in r57765

#25 @audrasjb
10 days ago

In 57792:

Toolbar: Fix dropdown admin menu styles on front-end.

This changeset switches back the admin menu items height property to px unit to prevent issues with themes using html { font-size: 62.5%; }.

Follow-up to [57765].

Props bgnicolepaschen, sabernhardt, huzaifaalmesbah, ironprogrammer, shailu25.
Fixes #60707.
See #43633.

#26 @audrasjb
7 days ago

In 57808:

Toolbar: Fix dropdown admin menu styles on front-end.

This changeset switches back the admin menu items height property to px unit to prevent issues with themes using html { font-size: 62.5%; }.

Follow-up to [57765].

Reviewed by swissspidy.
Merges [57792] to the to the 6.5 branch.

Props bgnicolepaschen, sabernhardt, huzaifaalmesbah, ironprogrammer, shailu25, mohonchandra.
Fixes #60707.
See #43633.

Note: See TracTickets for help on using tickets.