eBay / ebayui-core

Collection of Marko widgets; considered to be the core building blocks for all eBay components, pages & apps
https://ebay.github.io/ebayui-core/
Other
217 stars 101 forks source link

ebay-listbox-button: keyboard focus issue after shift-tabbing back to button from listbox #1778

Open ianmcburnie opened 1 year ago

ianmcburnie commented 1 year ago

Bug Report

eBayUI Version: 10.x

Description

  1. Goto: https://opensource.ebay.com/ebayui-core/?path=/story/buttons-ebay-listbox-button--standard
  2. Set keyboard focus on button
  3. Press ENTER or SPACEBAR
  4. Press SHIFT+TAB
  5. Press TAB

Result: listbox collapses, and focus moves to next focusable element on page. Expected: focus returns to listbox.

I think we just need to change the listbox to have tabindex=0 when in expanded state, rather than tabindex=-1, as in this example: https://makeup.github.io/makeup-js/makeup-listbox-button/index.html

Workaround

Yes. Move focus back to button and press ENTER or SPACEBAR again to re-open listbox.

agliga commented 1 year ago

This might be an issue with makeup-expander. I switched the tabindex to 0 in the code but expander is switching it to -1 https://github.com/makeup/makeup-js/blob/master/packages/makeup-expander/src/index.js#L89-L92 We might have to change the focusManagament.

saiponnada commented 1 year ago

Yes, I have discussed this with @ianmcburnie. Also another gap I noticed b/w make-up and ebay UI on focus management is make-up uses focusable where as eBay-ui uses content.

https://github.com/makeup/makeup-js/blob/master/packages/makeup-listbox-button/src/index.js#L37 https://github.com/eBay/ebayui-core/blob/master/src/components/ebay-listbox-button/component.js#L75

Imho, content makes more sense as listbox items doesn't have any interactive elements. So should we make following changes,

  1. remove L90 in makeup-expander (https://github.com/makeup/makeup-js/blob/master/packages/makeup-expander/src/index.js#L90 )
  2. change makeup-listbox-button to use focus management as content as well?