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

Multi Action Button can't close with Esc key #4522

Closed
1 task done
szeyingyau opened this issue Oct 28, 2021 · 15 comments · Fixed by #5235
Closed
1 task done

Multi Action Button can't close with Esc key #4522

szeyingyau opened this issue Oct 28, 2021 · 15 comments · Fixed by #5235

Comments

@szeyingyau
Copy link
Contributor

szeyingyau commented Oct 28, 2021

Current behaviour

The MultiActionButton cannot close when the Esc key is used.

Expected behaviour

The MultiActionButton should be able to close when the Esc key is used.

CodeSandbox or Storybook URL

https://codesandbox.io/s/autumn-wave-wg9gc?file=/src/index.js

JIRA Ticket (Sage Only)

No response

Suggested Solution

No response

Carbon Version

95.1.0

What browsers are you seeing the problem on?

Chrome, Firefox, Edge, Safari

What Operating System are you seeing the problem on?

MacOS

Anything else we should know?

No response

Confidentiality

  • I confirm there is no confidential or commercially sensitive information included.
@szeyingyau szeyingyau added Bug triage Triage Required labels Oct 28, 2021
@samtjo
Copy link
Contributor

samtjo commented Nov 2, 2021

@tempertemper would you mind having a look at this please?

If you tab onto the multi-action-button it will open, but there's no way to close it with the keyboard, other than tabbing away from it. Should we be able to press escape (or something else) to close the dropdown?

Thanks

@tempertemper
Copy link
Contributor

tempertemper commented Nov 18, 2021

esc should close the pop up and return focus to the trigger. Problem is, focusing the trigger automatically shows the dropdown menu…

I'm not sure what the right direction to take to fix this is, but I'm pretty sure it's design-led, and probably linked to the hover/focus triggering the dropdown menu, rather than behaving like a default button, which would take focus and provide a hover affordance, but would require a click/tap/enter/space to show the drop down. Maybe @harpalsingh can advise on the best course of action here; maybe a multi-action button task force.

Note for the 'task force': focus is returned to the parent button when tab is pressed on a sub-menu item. This should probably take the user to the next control on the interface, in order to satisfy 2.4.3 Focus Order.

@nicktitchmarsh nicktitchmarsh added Design System Review Required DS team to review changes and removed Pending a11y Review labels Nov 23, 2021
@nicktitchmarsh
Copy link
Contributor

Please can Design System team review Martin's comments here and advise on the behaviour for Multi Action Buttons

@samtjo
Copy link
Contributor

samtjo commented Feb 22, 2022

@clairedenning @harpalsingh do either of you know what we should do with this?

@clairedenning
Copy link

I think @andy varley would be the best person to answer this. Can't tag him here, I'll message him.

@tempertemper
Copy link
Contributor

I'm catching up with him on Friday morning so I'd be happy to fill him in 👍

@tempertemper
Copy link
Contributor

Tagging @v99rly

@samtjo
Copy link
Contributor

samtjo commented Apr 12, 2022

Hi @tempertemper - did Andy and yourself get anywhere with this?

@tempertemper
Copy link
Contributor

Thanks for the nudge, @samtjo. I caught up with him, but there's a good chunk of work in re-speccing the component, and I think the DS team have been struggling to prioritise non-brand-related work recently. I'll catch up with him on it when he returns from his holiday next week 👍

@nicktitchmarsh
Copy link
Contributor

FE-5188

@nicktitchmarsh
Copy link
Contributor

We have recently added work so the menu closes after an action has been used (Enter key), we could add the Esc key functionality as well.

@nicktitchmarsh nicktitchmarsh added On Backlog and removed triage Triage Required Design System Review Required DS team to review changes labels May 24, 2022
@tempertemper
Copy link
Contributor

Might be worth coming back to the Esc key functionality, since there could be the question of re-opening the menu, which would mean tabbing off then back onto the multi-action button. I'll address this with @v99rly again when I catch up with him on Friday.

@v99rly
Copy link

v99rly commented May 27, 2022

Had a chat with @tempertemper and checked what should be the correct behaviour. Will look into the with the design system team week commencing the 6th June.

@v99rly
Copy link

v99rly commented Jun 10, 2022

Please see ticket FE-5188 for further information

edleeks87 pushed a commit that referenced this issue Jun 16, 2022
Updates the keyboard navigation behaviour for `MultiActionButton`. Up and down
arrow keys can still be used for navigating children buttons but there is no longer
any looping back around. When the children container is open tabbing can now be used
to navigate the children buttons, the tab key takes you down the list and back tab
goes up. When the last child button is focused and tab is pressed it will close the
container and move focus to the next focusable element on the page. When the shift
and tab keys are pressed and the first child button is focused it will close the
container and return focus back to the main button control. Pressing the escape key
when the children container is open will now close the list and return focus back to
the main button control. All other keyboard functionality remains the same

fix #4522
edleeks87 pushed a commit that referenced this issue Jun 16, 2022
Updates the keyboard navigation behaviour for `SplitButton`. Up and down
arrow keys can still be used for navigating children buttons but there is no longer
any looping back around. When the children container is open tabbing can now be used
to navigate the children buttons, the tab key takes you down the list and back tab
goes up. When the last child button is focused and tab is pressed it will close the
container and move focus to the next focusable element on the page. When the shift
and tab keys are pressed and the first child button is focused it will close the
container and return focus back to the main button control. Pressing the escape key
when the children container is open will now close the list and return focus back to
the main button control. All other keyboard functionality remains the same

fix #4522
edleeks87 pushed a commit that referenced this issue Jun 16, 2022
Updates the keyboard navigation behaviour for `MultiActionButton`. Up and down
arrow keys can still be used for navigating children buttons but there is no longer
any looping back around. When the children container is open tabbing can now be used
to navigate the children buttons, the tab key takes you down the list and back tab
goes up. When the last child button is focused and tab is pressed it will close the
container and move focus to the next focusable element on the page. When the shift
and tab keys are pressed and the first child button is focused it will close the
container and return focus back to the main button control. Pressing the escape key
when the children container is open will now close the list and return focus back to
the main button control. All other keyboard functionality remains the same

fix #4522
edleeks87 pushed a commit that referenced this issue Jun 16, 2022
Updates the keyboard navigation behaviour for `SplitButton`. Up and down
arrow keys can still be used for navigating children buttons but there is no longer
any looping back around. When the children container is open tabbing can now be used
to navigate the children buttons, the tab key takes you down the list and back tab
goes up. When the last child button is focused and tab is pressed it will close the
container and move focus to the next focusable element on the page. When the shift
and tab keys are pressed and the first child button is focused it will close the
container and return focus back to the main button control. Pressing the escape key
when the children container is open will now close the list and return focus back to
the main button control. All other keyboard functionality remains the same

fix #4522
edleeks87 pushed a commit that referenced this issue Jun 27, 2022
Updates the keyboard navigation behaviour for `MultiActionButton`. Up and down
arrow keys can still be used for navigating children buttons but there is no longer
any looping back around. When the children container is open tabbing can now be used
to navigate the children buttons, the tab key takes you down the list and back tab
goes up. When the last child button is focused and tab is pressed it will close the
container and move focus to the next focusable element on the page. When the shift
and tab keys are pressed and the first child button is focused it will close the
container and return focus back to the main button control. Pressing the escape key
when the children container is open will now close the list and return focus back to
the main button control. All other keyboard functionality remains the same

fix #4522
edleeks87 pushed a commit that referenced this issue Jun 27, 2022
Updates the keyboard navigation behaviour for `SplitButton`. Up and down
arrow keys can still be used for navigating children buttons but there is no longer
any looping back around. When the children container is open tabbing can now be used
to navigate the children buttons, the tab key takes you down the list and back tab
goes up. When the last child button is focused and tab is pressed it will close the
container and move focus to the next focusable element on the page. When the shift
and tab keys are pressed and the first child button is focused it will close the
container and return focus back to the main button control. Pressing the escape key
when the children container is open will now close the list and return focus back to
the main button control. All other keyboard functionality remains the same

fix #4522
edleeks87 pushed a commit that referenced this issue Jul 18, 2022
Updates the keyboard navigation behaviour for `MultiActionButton`. Up and down
arrow keys can still be used for navigating children buttons but there is no longer
any looping back around. When the children container is open tabbing can now be used
to navigate the children buttons, the tab key takes you down the list and back tab
goes up. When the last child button is focused and tab is pressed it will close the
container and move focus to the next focusable element on the page. When the shift
and tab keys are pressed and the first child button is focused it will close the
container and return focus back to the main button control. Pressing the escape key
when the children container is open will now close the list and return focus back to
the main button control. All other keyboard functionality remains the same

fix #4522
edleeks87 pushed a commit that referenced this issue Jul 18, 2022
Updates the keyboard navigation behaviour for `SplitButton`. Up and down
arrow keys can still be used for navigating children buttons but there is no longer
any looping back around. When the children container is open tabbing can now be used
to navigate the children buttons, the tab key takes you down the list and back tab
goes up. When the last child button is focused and tab is pressed it will close the
container and move focus to the next focusable element on the page. When the shift
and tab keys are pressed and the first child button is focused it will close the
container and return focus back to the main button control. Pressing the escape key
when the children container is open will now close the list and return focus back to
the main button control. All other keyboard functionality remains the same

fix #4522
edleeks87 pushed a commit that referenced this issue Jul 19, 2022
Implements `useMenuKeyboardNavigation` to Update the keyboard navigation behaviour
for `MultiActionButton`. Up and down arrow keys can still be used for navigating
children buttons but there is no longer any looping back around. When the children
container is open tabbing can now be used to navigate the children buttons, the tab
key takes you down the list and back tab goes up. When the last child button is
focused and tab is pressed it will close the container and move focus to the next
focusable element on the page. When the shift and tab keys are pressed and the first
child button is focused it will close the container and return focus back to the main
button control. Pressing the escape key when the children container is open will now
close the list and return focus back to the main button control. All other keyboard
functionality remains the same

fix #4522
edleeks87 pushed a commit that referenced this issue Jul 19, 2022
Implements `useMenuKeyboardNavigation` to update the keyboard navigation behaviour
for `SplitButton`. Up and down arrow keys can still be used for navigating children
buttons but there is no longer any looping back around. When the children container
is open tabbing can now be used to navigate the children buttons, the tab key takes
you down the list and back tab goes up. When the last child button is focused and
tab is pressed it will close the container and move focus to the next focusable
element on the page. When the shift and tab keys are pressed and the first child
button is focused it will close the container and return focus back to the main
button control. Pressing the escape key when the children container is open will
now close the list and return focus back to the main button control. All other
keyboard functionality remains the same

fix #4522
carbonci pushed a commit that referenced this issue Jul 19, 2022
### [109.3.1](v109.3.0...v109.3.1) (2022-07-19)

### Bug Fixes

* **multi-action-button:** add new keyboard functionality to component ([a76819b](a76819b)), closes [#4522](#4522)
* **split-button:** add new keyboard functionality to component ([5121f6a](5121f6a)), closes [#4522](#4522)
@carbonci
Copy link
Collaborator

🎉 This issue has been resolved in version 109.3.1 🎉

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
Development

Successfully merging a pull request may close this issue.

7 participants