WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#32740 closed defect (bug) (fixed)

Menu Customizer: "quick delete" focus

Reported by: afercia Owned by: valendesigns
Milestone: 4.3 Priority: normal
Severity: normal Version: 4.3
Component: Customize Keywords: has-patch commit
Focuses: ui, accessibility, javascript Cc:

Description

Noticed a couple of issues with the "Quick delete" red "X" buttons:

1.
The focus style should be more visible, at the moment there's only a slight change in the red color, see screenshot below where the "Custom Link" one is focused

2.
When activating the button with the keyboard (Enter or Space bar) focus is lost: the previously focused element just got removed from the DOM. Especially noticeable with Chrome, Firefox is smarter and always tries to keep focus "in place". Not sure about a proper solution here, maybe the focus should be moved to a fallback element (i.e. the "Add Items" button)? Quoting Mr. Marco Zehe from a post about a different scenario (dialogs) but the focus issue is the same:

In the worst case, (focus) lands on the document or somewhere else far away from where the user’s context was before ... There is nothing more frustrating than to have to spend 10 or more seconds trying to find one’s last place again from where one left off.

https://cldup.com/zMFAvmMN0b.png

Attachments (1)

32740.diff (2.4 KB) - added by valendesigns 5 years ago.

Download all attachments as: .zip

Change History (13)

#1 follow-up: @celloexpressions
5 years ago

Those previously had a box-shadow outline that I specifically remember adding, so that got reverted at some point. Also, it should be automatically focusing the next or previous delete button when deleted, if it's not doing that anymore then that was also reverted at some point.

No idea what would have caused these regressions.

#2 in reply to: ↑ 1 @afercia
5 years ago

Replying to celloexpressions:

Also, it should be automatically focusing the next or previous delete button when deleted

Noticed sometimes it does that, sometimes doesn't. No idea why.

#3 @afercia
5 years ago

Testing with a menu with 3 items, their IDs are: 2623, 2625, 2626. Please refer to the screenshot below, where in _setupRemoveUI I'm just printing to the console the prev and next items: console.log( control.container.prev(), control.container.next() );

First scenario:
deleting the items in order from the first to the last one. As you can see, when deleting 2625, the previously deleted item is still returned even if it was just deleted. Same when deleting the last one 2626. In this last case it will try to set focus on the previous item which doesn't exist any more in the DOM. Focus is lost.

Second scenario:
deleting the second item and then the first item: at this point will return 2625 as "next" but that item was just removed from the DOM. Focus is lost.

Things get a bit more complicated when you've just added new menu items to the menu.

Sorry I'm not so familiar with the Customizer to get exactly what's happening here, I just suspect debouncedReflowMenuItems() added here https://github.com/voldemortensen/menu-customizer/commit/a9d502b31f3f96758287390fa225e48959e274c6 has something to do with this.

https://cldup.com/0ziM0dVVjg.png

#4 @afercia
5 years ago

Looks like when deleting items reflowMenuItems() shouldn't do this:

menuItemControl.priority.set( context.currentAbsolutePosition ); // This will change the sort order.

#5 @valendesigns
5 years ago

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

#6 @obenland
5 years ago

@valendesigns, have you had a chance to look at this? You can always pass it on if you won't be able to find time for it.

#7 @valendesigns
5 years ago

@obenland Sorry about that, I got sick the last half of the week. I'll work on it now.

@valendesigns
5 years ago

#8 @valendesigns
5 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

I've added indication in the CSS that the item is focused. The issue with the JS was that it was not taking into consideration that the next or previous menu item may be hidden (removed) and not looking past that item for a visible one. Patch 32740.diff makes sure to look beyond the next or previous item when they are not visible before giving up the search and should solve all the issues mentioned above. Cheers!

#9 @celloexpressions
5 years ago

Patch looks good to me. I think we probably lost the box-shadow when this was changed from a link to a button.

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


5 years ago

#11 @westonruter
5 years ago

  • Keywords commit added; needs-testing removed

#12 @westonruter
5 years ago

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

In 33279:

Customizer: Fix element focus after menu item is deleted for keyboard accessibility.

Also restores box shadow on focus.

Props valendesigns.
Fixes #32740.

Note: See TracTickets for help on using tickets.