WordPress.org

Make WordPress Core

Opened 2 months ago

Closed 2 months ago

Last modified 2 months ago

#42114 closed task (blessed) (fixed)

Customize Menus: UX Improvements

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

Description

  • 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:

https://gist.github.com/melchoyce/fc28e4b6c643e85331779bfce0b006d8

Attachments (5)

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

Change History (29)

#1 @melchoyce
2 months ago

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

#2 @melchoyce
2 months ago

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

#3 follow-up: @afercia
2 months 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
2 months 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
2 months ago

@melchoyce how does this look to you?

#6 @melchoyce
2 months ago

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

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

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

#10 @bpayton
2 months ago

  • Keywords has-patch added

I created a PR for this here:
https://github.com/xwp/wordpress-develop/pull/284

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
2 months ago

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

#12 @bpayton
2 months 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
2 months 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.
https://github.com/xwp/wordpress-develop/pull/284

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).

Thanks!

#14 follow-up: @westonruter
2 months 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 WordPress.com 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
2 months ago

Replying to westonruter:

[snip]
But, if I recall correctly, the Customizer on WordPress.com 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!

@bpayton
2 months ago

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


2 months ago

#21 @Kopepasah
2 months 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
2 months 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.

@bpayton
2 months ago

Fix to avoid re-animation cases

#23 @bpayton
2 months 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
2 months 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.