Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#42114 closed task (blessed) (fixed)

Customize Menus: UX Improvements

Reported by: melchoyce's profile melchoyce Owned by: bpayton's profile bpayton
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.9
Component: Customize Keywords: has-patch
Focuses: ui Cc:


  • Improve UX by initially focusing name field in New Menu section
  • Improve UX by initially focusing or otherwise drawing attention to Add Items button after creating a menu

We could try using the animation currently used on the Publish button in the Customizer:

Attachments (5)

new-menu-item-highlight-20171012a.gif (2.2 MB) - added by bpayton 7 years ago.
new-menu-item-highlight-20171012b.gif (3.4 MB) - added by bpayton 7 years ago.
new-menu-item-highlight-20171013a.gif (2.5 MB) - added by bpayton 7 years ago.
42114.1.diff (5.4 KB) - added by bpayton 7 years ago.
42114.2.diff (1.2 KB) - added by bpayton 7 years ago.
Fix to avoid re-animation cases

Change History (29)

#1 @melchoyce
7 years ago

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

#2 @melchoyce
7 years ago

  • Type changed from defect (bug) to task (blessed)

#3 follow-up: @afercia
7 years ago

Not sure the initial keyboard focus should be set on the name field and then on the add items button. Initial focus in the Customizer is now consistent and always goes on the "Back" button. For consistency and accessibility I'd recommend to keep it as is.

I'd rather see the otherwise drawing attention part as an added value :) I understand the need to guide users in the menu creation flow. However, the UI is not designed as a multi-step wizard in the first place.
Seems to me in this flow there's a specific task to accomplish so I'd consider to make the "Next" and "Add Items" buttons look like primary buttons because they identify the primary action in this flow.

#4 in reply to: ↑ 3 @melchoyce
7 years ago

Replying to afercia:

I'd rather see the otherwise drawing attention part as an added value :)

Let's start with the delayed shaking animation, then. Maybe wait 2s, then shake once.

#5 @bpayton
7 years ago

@melchoyce how does this look to you?

#6 @melchoyce
7 years ago

@bpayton Can we use that same animation, but make it shake vertically instead of horizontally?

#7 @bpayton
7 years ago

@melchoyce Here's an update with a vertical animation. I reduced the shake slightly because it felt excessive relative to the height, but it might not be enough now. What do you think?

#8 @melchoyce
7 years ago

Looks good. Let's make sure that we update the Publish shake animation, in that case, so they both match. Actually, could they even share the same keyframes?

#9 @bpayton
7 years ago

Thanks for bringing that up. We have a button-see-me class that will work well.

#10 @bpayton
7 years ago

  • Keywords has-patch added

I created a PR for this here:

I added a reusable util function for pointing out a button after a delay if the user doesn't focus it first. I plan to use it to address #42211.

#11 @bpayton
7 years ago

I need to add test(s) for the reusable function but am interested in any feedback in the meantime.

#12 @bpayton
7 years ago

I'd added the function to api.utils but think doing so is a mistake. I don't want to create a new, public function but would like to share a utility function internally. @westonruter, where might be a good place for this?

#13 @bpayton
7 years ago

I made updates to better accommodate other scenarios where we shake UI to draw the user's attention (#42211). It would be good to confirm the general direction before writing tests for the reusable function.

Here's a PR where I'm managing the work. I've been working this way with Weston but am happy to provide a patch if another reviewer prefers.

A couple of concerns:

  • Finding a good place to share an internal function across files without adding to the public namespace api.utils.
  • Naming: I don't like the names I used but haven't thought of better (e.g., all names beginning with pointOut).


#14 follow-up: @westonruter
7 years ago

@bpayton I can't think of a better place to add this function than wp.customize.utils, since you're using it across multiple files. If you want to keep it private, then I suggest prefixing with an underscore and marking @private. But, if I recall correctly, the Customizer on has some of the same kind of bouncing effects; so it seems to me that this kind of function should be available for use outside of core?

As for naming, you might want to consider “highlight” terminology. This is used in customize-widgets.js.

#15 in reply to: ↑ 14 @bpayton
7 years ago

Replying to westonruter:

But, if I recall correctly, the Customizer on has some of the same kind of bouncing effects; so it seems to me that this kind of function should be available for use outside of core?

I agree this kind of function could be useful outside of core. The reason I wanted to define it as an internal function is that I'm not 100% sure this is the right signature and behavior for general use, and it's much easier to change an internal function after the fact. I'm not against making it available though.

As for naming, you might want to consider “highlight” terminology. This is used in customize-widgets.js.

I'd considered it and was initially uncomfortable because I imagined a function like highlightElement doing the highlighting with colors rather than animation, but I don't think it matters. Both are simply drawing the user's attention. I'll make the switch. Thanks!

7 years ago

#16 @bpayton
7 years ago

I uploaded a patch containing the main change. The change tests well for me in Chrome, Firefox, and IE11.

We need unit tests, and I'm working on them but am hitting odd qunit errors at the moment.

#17 @westonruter
7 years ago

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

In 41930:

Customize: Draw attention to "Add Items" button after creating a new menu.

Introduce wp.customize.utils.highlightButton() and wp.customize.Menus.MenuSection#highlightNewItemButton().

Props bpayton, melchoyce, afercia, westonruter.
Fixes #42114.

#18 @westonruter
7 years ago

In 41932:

Customize: Draw attention to save button after collapsing the publish settings section when the changeset status or date had been changed.

Props bpayton, westonruter.
See #42114, #39896.
Fixes #42211.

#19 @westonruter
7 years ago

@bpayton I'm seeing a small issue here. I am creating a new menu and the “Add Items” button shakes after 2 seconds as expected. However, then every time I add an item it shakes every time one is added. Shouldn't it stop shaking after I open the available items pane?

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

7 years ago

#21 @Kopepasah
7 years ago

@westonruter I am seeing this issue too. It seems that the cancel of the highlight on the button is bound to the publish button's cancel. Seems that this should be bound to adding a new menu item to the menu.

#22 @bpayton
7 years ago

@westonruter I am able to reproduce this as well. I believe this is due to the button-see-me class applied to the button for animation combined with calls to MenuControl#reflowMenuItems. The issue is also visible when items are deleted for a new menu.

I will look into fixing this by removing the button-see-me class on animationend.

7 years ago

Fix to avoid re-animation cases

#23 @bpayton
7 years ago

I attached a patch with a fix.

It isn't perfect in that an animationend listener can remain attached to the highlighted button if anything interrupts animation. Both removing the animation class and removing the animated element from the DOM can interrupt animation. But a subsequent completed animation will remove the listener. I believe this is reasonable compromise over adding an additional cleanup attempt with setTimeout and a sufficient delay.

#24 @westonruter
7 years ago

In 41950:

Customize: Prevent re-highlighting "Add Items" button after available nav menu items pane has already been opened.

Amends [41930].
Props bpayton.
See #42114.

Note: See TracTickets for help on using tickets.