Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ianmcburnie opened this issue Oct 3, 2022 · 2 comments
Assignees

Comments

@ianmcburnie
Copy link
Contributor

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 agliga added this to To do in 10.2.0 via automation Oct 19, 2022
@saiponnada saiponnada moved this from To do to In progress in 10.2.0 Oct 24, 2022
@agliga
Copy link
Contributor

agliga commented Oct 26, 2022

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
Copy link
Contributor

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?

@saiponnada saiponnada moved this from In progress to To do in 10.2.0 Nov 7, 2022
@agliga agliga removed this from To do in 10.2.0 Nov 9, 2022
@agliga agliga added this to To do in 11.0.0 via automation Nov 9, 2022
@agliga agliga removed this from To do in 11.0.0 Dec 2, 2022
@agliga agliga added this to To do in 11.1.0 via automation Dec 2, 2022
@agliga agliga removed this from To do in 11.1.0 Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

3 participants