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

ActionPopover - opening with Up Arrow focuses first element rather than last #6826

Closed
1 task done
robinzigmond opened this issue Jul 11, 2024 · 5 comments · Fixed by #7003
Closed
1 task done

ActionPopover - opening with Up Arrow focuses first element rather than last #6826

robinzigmond opened this issue Jul 11, 2024 · 5 comments · Fixed by #7003

Comments

@robinzigmond
Copy link
Contributor

Description

From the code in the ActionPopover component, it is apparent that pressing the up arrow key is supposed to open the menu with the last focusable item focused. We have:

Yet not only does an RTL unit test for this behaviour fail, it fails when tested in the browser in our Storybook docs.

Reproduction

https://carbon.sage.com/?path=/docs/action-popover--docs

Steps to reproduce

Tab to menu button and open by pressing the up arrow. The first item is focused, rather than the last.

JIRA ticket numbers (Sage only)

No response

Suggested solution

Confirm with UX/accessibility whether this behaviour (focusing the last element) is actually wanted or not (there is a comment in the code referring to ARIA best practices and referencing https://www.w3.org/WAI/ARIA/apg/patterns/menu-button/examples/menu-button-actions/, but that example focuses the first element, not the last, when opened with the up arrow key).

If focus-last-element is correct, then we should fix the bug, and uncomment the commented-out RTL test that I am adding for this.

If focus-first-element is correct, then the existing code and tests which claim to implement this should be removed, and tests added to ensure that the first element is focused instead.

Carbon version

latest

Design tokens version

No response

Relevant browsers

Chrome

Relevant OSs

MacOS

Additional context

No response

Confidentiality

  • I confirm there is no confidential or commercially sensitive information included.
@edleeks87
Copy link
Contributor

@tempertemper @ljemmo @harpalsingh would you be able to steer on what the desired behaviour is here please?

@ljemmo
Copy link
Contributor

ljemmo commented Aug 6, 2024

@edleeks87 Having had a play around with other products and design systems, i think the desired behaviour would be to indeed open the last item as opposed to the first when the up arrow is pressed. Any objections @harpalsingh and @tempertemper?

@nicktitchmarsh
Copy link
Contributor

FE-6793

@nicktitchmarsh nicktitchmarsh added On Backlog and removed triage Triage Required labels Sep 3, 2024
@tempertemper
Copy link
Contributor

On balance, I think removing the up/down-to-open the menu is perhaps the right direction. Just a simple button press (click, Enter, Space, etc.) to open/close.

The up/down behaviour was either left in or added as an enhancement to the standard keyboard behaviour (I was part of this conversation and saw no issue), but I wonder if we're trying too hard. Just let the button behave like a button.

That would also mean we're not hijacking scroll for keyboard users who focus on the button and don't want to interact with it, but instead want to scroll the page using the arrow keys (default behaviour).

DipperTheDan added a commit that referenced this issue Oct 8, 2024
…ast element in the menu

Currently we have a bug whereby when a user presses the up arrow key, the first focusable item in
the menu is focused. This appears to be a bug as we have code that specifically handles this
behaviour but is overridden elsewhere in the code. This fix ensures that the last focusable item is
focused.

fixes #6826
carbonci pushed a commit that referenced this issue Oct 18, 2024
### [143.2.4](v143.2.3...v143.2.4) (2024-10-18)

### Bug Fixes

* **action-popover:** ensure that opening using the up arrow focuses last element in the menu ([38aaed9](38aaed9)), closes [#6826](#6826)
@carbonci
Copy link
Collaborator

🎉 This issue has been resolved in version 143.2.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment