WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#36795 closed task (blessed) (fixed)

Customize: Improve flow from menu locations to editing a menu

Reported by: celloexpressions Owned by: helen
Milestone: 4.6 Priority: normal
Severity: normal Version: 4.3
Component: Customize Keywords: has-patch has-screenshots commit
Focuses: ui, accessibility Cc:
PR Number:

Description

From the menu locations section, it may be advantageous to be able to directly navigate to editing the menu assigned to a particular location, rather than having to go back to the list of menus and find the menu assigned to the location in question. This can be accomplished fairly easily by adding an "edit" button to each menu location, and linking it to the corresponding menu section.

Attachments (15)

36795.PNG (18.2 KB) - added by celloexpressions 4 years ago.
36795.diff (1.9 KB) - added by celloexpressions 4 years ago.
36795.gif (552.3 KB) - added by celloexpressions 3 years ago.
With 36795.diff.
36795.2.diff (1.6 KB) - added by afercia 3 years ago.
36795.3.diff (1.6 KB) - added by ocean90 3 years ago.
36795.4.diff (514 bytes) - added by celloexpressions 3 years ago.
Improve vertical positioning and increase tappable/clickable area. Match select element height and font size.
line-height-before.png (41.4 KB) - added by ocean90 3 years ago.
line-height-after.png (31.2 KB) - added by ocean90 3 years ago.
36795.5.diff (425 bytes) - added by ocean90 3 years ago.
win10-edge-before.png (4.3 KB) - added by ocean90 3 years ago.
win10-edge-after.png (4.3 KB) - added by ocean90 3 years ago.
win10-ff-before.png (3.3 KB) - added by ocean90 3 years ago.
win10-ff-after.png (3.5 KB) - added by ocean90 3 years ago.
win10-chrome-before.png (5.3 KB) - added by ocean90 3 years ago.
win10-chrome-after.png (5.4 KB) - added by ocean90 3 years ago.

Download all attachments as: .zip

Change History (58)

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


3 years ago

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


3 years ago

#3 @jaragozzine
3 years ago

I like this idea very much, from a non-technical, convenience factor. We leverage the Customizer screen for all sorts of editorial site control often enough.

That said, the menu editing UI takes up a lot of screen real estate; beyond the drop-downs shown, how do you suggest we handle the rest of the necessary screen real estate (list of pages, categories, etc; drag-and-drop menu builder; will this be previewed like other Customizer elements?)

From a developer perspective (which I do not have) I wonder if there is any technical "heavy lifting" we'd run into by attempting this?

Thanks - looking forward to seeing what you come up with next!

#4 @celloexpressions
3 years ago

@jaragozzine this would link to the existing menu section in the customizer with the Section.focus() method. 36795.diff includes everything that is needed and has minimal complexity relative to the rest of the customizer menus code.

#5 @chriscct7
3 years ago

  • Owner set to celloexpressions
  • Status changed from new to assigned

#6 @celloexpressions
3 years ago

  • Keywords ux-feedback added

This should be ready for a committer to look at, although I don't think @westonruter has time. Wouldn't hurt to discuss at this week's design meeting as well cc @karmatosed @hugobaeta (I can't make it but would appreciate any feedback).

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


3 years ago

#8 @celloexpressions
3 years ago

  • Keywords ux-feedback removed

Feedback in the design meeting was positive, although no one has had a chance to test the patch yet. Once that happens this should be ready for commit.

#9 @celloexpressions
3 years ago

  • Owner changed from celloexpressions to westonruter
  • Status changed from assigned to reviewing

Not a lot of feedback but positive so far so let's go for it. Assigning for patch review/commit.

This ticket was mentioned in Slack in #core by ocean90. View the logs.


3 years ago

#11 @ocean90
3 years ago

  • Owner changed from westonruter to ocean90

#12 @ocean90
3 years ago

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

In 37901:

Customize: Improve flow from menu locations to editing a menu.

Adds an Edit button beside the menu location to allow users to switch to the selected menu for further edits.

Props celloexpressions.
Fixes #36795.

#13 @afercia
3 years ago

  • Focuses accessibility added
  • Keywords 2nd-opinion added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for a few improvements consideration:

Maybe it's just me, open to different opinions and any thoughts welcome, but the current buttons suggest the idea of a "Select" or "Apply" action and maybe they're potentially confusing for users:

https://cldup.com/WeljgWMpTt.png

Also, for consistency with the nav-menus.php menu locations screen (even if the "Edit" functionality there works a bit differently)...

https://cldup.com/xSsDirhG6V.png

...I'd consider to make these buttons look like links:

https://cldup.com/bmIq87xJZ8.png

Lastly, for accessibility, they should provide additional information: when read out of context it's not clear what to "Edit":

https://cldup.com/JeYMEdL2yp.png

maybe worth considering to add an aria-label attribute with a value, to keep it simple, "Edit selected menu" or something like that.

P.S. please, please, always mark with the "accessibility" focus any ticket that introduces UI changes, especially about UI controls and form elements.

This ticket was mentioned in Slack in #core by afercia. View the logs.


3 years ago

#15 @ocean90
3 years ago

@afercia Can you work on a patch for your suggestions?

@afercia
3 years ago

#16 @afercia
3 years ago

  • Keywords 2nd-opinion removed

36795.2.diff uses the button-link CSS class and adds a few CSS rules to make button-link work nicely, see #35126. Also, adds an aria-label attribute to give more contextual information.

Screenshot with the button-links in normal, hover, and focus state:

https://cldup.com/5k4QpjYU1D.png

Edit: forgot to mention that the default link color #0073aa on the Customizer background #eeeeee has a contrast ratio of 4.49:1 which substantially meets the 4.5:1 required ratio. It would just need that missing 0.01 :) by the way this should be addressed in the ongoing effort to improve contrast ratio and reduce the shades of grey in the admin.

Last edited 3 years ago by afercia (previous) (diff)

#17 @ocean90
3 years ago

  • Keywords commit i18n-change added
  • Owner changed from ocean90 to westonruter
  • Status changed from reopened to reviewing

36795.2.diff looks good to me. Although the string for the aria label is not new it will cause an i18n-change because the string gets moved into the front end project.

@celloexpressions Any objections here?

@westonruter If @celloexpressions has no objections, can I get your sign off?

This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.


3 years ago

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


3 years ago

#20 follow-up: @celloexpressions
3 years ago

  • Keywords ui-feedback added; commit removed

We should absolutely add an aria-label. I think the title of the selected menu may also be useful here, although it presumably would have just been read out with the selected control. It would also need to be updated when the title is edited and that starts to get a bit messy.

I'm hesitant to change anything visually here at this point. We're in RC, and this is the first time it's come up as an issue in 5 weeks. Granted there hasn't been a lot of feedback here to date in general; it isn't really "broken" visually, but we could perhaps try something different. CSS changes this late should be avoided unless absolutely necessary due to the likelihood of inadvertently breaking something.

I agree that the placement and styling of the button could lead users to think that it's meant to submit something. However, there aren't many inputs in the customizer that have submit buttons next to them, and clicking to edit the menu isn't the end of the world and could even be argued as expected "submit" behavior given the way things work in the customizer. The flow is to select a menu, then edit it as it becomes visible in the preview at the same time.

In terms of the visual appearance of this as an underlined link, I'm not a fan. The contrast is quantitatively low (and primary actionable UI like this should generally have higher contrast that regular text, etc.), the font size is extremely small (especially with the native Windows font), and the underline combined with the small font size and the short width of the word (in English at least) make it quite difficult to read. It looks like we're trying to hide the fact that there's UI here.

Brief aside: the accessibility team should probably follow the UI focus to track everything that changes UI, and use the accessibility focus for tickets focused primarily on accessibility. Otherwise, the two focuses are redundant.

#21 in reply to: ↑ 20 @afercia
3 years ago

Replying to celloexpressions:

I'm hesitant to change anything visually here at this point. We're in RC, and this is the first time it's come up as an issue in 5 weeks.

That's because no one was aware of this ticket I guess. It was originally marked with the "UI" focus but we're still waiting for some UI feedback. It was not originally marked with the "accessibility" focus so there was no feedback.

I agree that the placement and styling of the button could lead users to think that it's meant to submit something.

Yep, this is my main concern. I'm not married to the proposed change but I'd like to hear some feedback from the UI team though.

Brief aside: the accessibility team should probably follow the UI focus to track everything that changes UI, and use the accessibility focus for tickets focused primarily on accessibility. Otherwise, the two focuses are redundant.

It's an interesting proposal, I guess the accessibility team would be happy to have new contributors to handle the huge amount of work required to follow all the things. Volunteers welcome :) Sometimes, proposed UI changes are just "visual" though, and don't impact accessibility. So I'm not sure following the UI focus would help. On the other hand, marking a ticket with the right "focus" it's a matter of just a few clicks.

#22 @zetaraffix
3 years ago

Considering UI as a language: giving same "words" different meaning leads to confusion, especially on a ​"less-than-aware"​ level of action (we learn to give certain things certain meanings, and often operate on them automatically).

Consistency is very powerful. We do "mind shortcuts" and consistency allows for fewer mistakes. Which is why I think a link is a better choice in this situation. :)

#23 @karmatosed
3 years ago

A link fits in pattern wise, more than a button.

#24 @ocean90
3 years ago

  • Keywords commit added; ui-feedback removed

Re-adding commit for 36795.2.diff based on the feedback by the design team.

#25 @afercia
3 years ago

Adding some screenshots taken on Win 10 with latest Chrome, Firefox, IE 11, and OS X with Chrome. As you can see alignments are different, especially on Firefox/Win 10. This is because of native fonts as noted in the related ticket, see #36753.

Re: contrast issues expressed for example on Slack, see https://wordpress.slack.com/archives/design/p1470184543000460 I share the same concern but I'd see this as a general issue in the Customizer; for example, it applies also to the "widget areas" link a few line above. It should probably be addressed as a general Customizer UI issue, not specifically for these two links. I know the Customizer team is discussing contrast and UI hierarchy and there's also an ongoing discussion in the Design team about contrast, shades of grey, and links color. I'd suggest some collaboration between the two teams, trying to focus on the big picture and not on specific cases.

The perceived smaller font size, especially on Windows, is again an effect of the switch to native fonts. My personal opinion is we're in 2016 and a 13 pixels default font-size maybe is starting to be a bit too small. Currently, GitHub uses the same native fonts stack and if I'm not wrong the main sizes are 14 and 16 pixels (except buttons). I'd like to see some discussion about this in the next release cycles.

What can be done to improve a bit these 2 links at this point of the release? I'd propose:

  • rather than increasing the font size, I'd increase the clickable area with some padding
  • try to improve a bit the alignments, even if it's very unlikely they can be perfect across different platforms/browsers because of the native fonts

Win 10 chrome

https://cldup.com/an0nI3iad2.png

Win 10 firefox

https://cldup.com/k9LtEiUe0q.png

Win 10 ie11

https://cldup.com/I3w7dbZE5Y.png

OS X chrome

https://cldup.com/xWawbTvXvv.png

#26 follow-up: @helen
3 years ago

The link seems more appropriate to me because you're basically taken away from that view, though much more closely related than a complete page load. I agree that it doesn't stand out as much visually, but at the same time, it's not really the focus of what you're doing there, and as a button next to a dropdown it sort of implies some kind of saving, despite saying "Edit".

My hesitation is much more around whether "Edit" makes sense there in the visual version - "Edit selected menu" is too long of a string for the space, but perhaps "Edit Menu"? I know it's taking cues from the admin screen, but that doesn't make that a good thing. I find myself wondering what exactly I'm going to be editing (the choices available in that dropdown? The selected menu? The menu location?), even though I know what's going on.

#27 in reply to: ↑ 26 @melchoyce
3 years ago

Replying to helen:

"Edit Menu"

+1

@ocean90
3 years ago

#28 follow-up: @ocean90
3 years ago

@afercia Should the the aria label remain even if the button label is "Edit Menu", see 36795.3.diff?

"Edit Menu" is an existing string.

#29 in reply to: ↑ 28 @afercia
3 years ago

Replying to ocean90:

@afercia Should the the aria label remain even if the button label is "Edit Menu"

I'd say yes, since it seems the most appropriate wording for this control it would be nice to use it at least for assistive technologies. I understand the will to avoid a string change in RC though so up to you, maybe can be moved to a separate ticket.

Replying to melchoyce:

"Edit Menu"

+1

+1 and maybe it should be updated also in nav-menus.php.

#30 @ocean90
3 years ago

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

In 38189:

Customize: Make the menu edit button look like a link.

The button was introduced in [37901] to allow users switching to the selected menu for further edits. A link makes it more clear that the user is taken away from the current view.
This also adds an aria label and makes the button label more verbose, 'Edit Menu'.

Props afercia.
Props helen for review.
Fixes #36795.

#31 @ocean90
3 years ago

In 38190:

Customize: Make the menu edit button look like a link.

The button was introduced in [37901] to allow users switching to the selected menu for further edits. A link makes it more clear that the user is taken away from the current view.
This also adds an aria label and makes the button label more verbose, 'Edit Menu'.

Merge of [38189] to the 4.6 branch.

Props afercia.
Props helen for review.
See #36795.

@celloexpressions
3 years ago

Improve vertical positioning and increase tappable/clickable area. Match select element height and font size.

#32 @celloexpressions
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

As @afercia suggested in 25, we need to provide padding on the link so that it is accessible to touch users. This would also improve the legibility of the focus style. [38189] is a usability regression for touch users.

My suggestion would be to do this with line-height instead of padding, as that should provide more reliable/consistent results than vertical-align: middle. This allows us to match the explictily-defined 28px height of the adjacent select element.

36795.4.diff also adjusts the font-size to match the 14px of the select element and customize control title, although that part is more subjective and could be excluded for 4.6. Arguments could be made for either 13px or 14px, but it seems like it should match the font in the select next to it and the title directly above it rather than the section description up above.

@ocean90
3 years ago

#33 @ocean90
3 years ago

  • Keywords i18n-change commit removed
  • Type changed from enhancement to task (blessed)

[38189] is a usability regression for touch users.

Not sure I agree here. I just tried it and had a hard time to *not* hit the button.

Anyway, font size shouldn't be part of this so 36795.5.diff adds only the line height.

#34 @ocean90
3 years ago

  • Owner westonruter deleted
  • Status changed from reopened to reviewing

This ticket was mentioned in Slack in #core by helen. View the logs.


3 years ago

#36 @ocean90
3 years ago

  • Keywords commit added

#37 @ocean90
3 years ago

  • Owner set to helen

#38 @afercia
3 years ago

I guess it's not a blocker, but checking this on Win 10 with the main 4 browsers, the alignment doesn't look so pretty as on OS X. For a mix of reasons, different fonts, different OS select rendering, etc. I've played a bit with this and I think it's something that can be fixed but probably not on this ticket. It would require adjustments to the select styling and maybe this is something that should be done for all the WP form elements, hopefully in the next release cycle.

This ticket was mentioned in Slack in #core by ocean90. View the logs.


3 years ago

#40 @helen
3 years ago

Go for it.

#41 @ocean90
3 years ago

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

In 38207:

Customize: Increase the target size of the 'Edit Menu' button.

Props celloexpressions.
Props afercia, helen for review.
Fixes #36795.

#42 @ocean90
3 years ago

In 38208:

Customize: Increase the target size of the 'Edit Menu' button.

Merge of [38207] to the 4.6 branch.

Props celloexpressions.
Props afercia, helen for review.
See #36795.

This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.


3 years ago

Note: See TracTickets for help on using tickets.