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.

Sign up for GitHub

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

Jump to bottom

[Floating Menu] Focus wrap should work without having to put in extra elements #3603

Closed
1 of 2 tasks
elizabethsjudd opened this issue Jul 31, 2019 · 9 comments · Fixed by #3652
Closed
1 of 2 tasks

[Floating Menu] Focus wrap should work without having to put in extra elements #3603

elizabethsjudd opened this issue Jul 31, 2019 · 9 comments · Fixed by #3652
Assignees
@elizabethsjudd
Labels
type: bug 🐛

Comments

@elizabethsjudd
Copy link
Contributor

@asudoh @dakahn @snidersd

What package(s) are you using?

  • carbon-components
  • carbon-components-react

Detailed description

Describe in detail the issue you're having.

Demos are deceiving because there is a visually hidden input that is not part of the "demo" code but this input is required in order to have the a11y focus wrap feature work. I understand that any "focusable" element following a floating menu would work and it does not have to be this specific input but we should not assume that of our users' use-cases. Focus wrap should work without having to add in extra elements, especially if it's something like this input which is visually hidden but is visible to screen reader users, making it really confusing as to why they are reaching a text input.

Is this issue related to a specific component?

Any component that uses Floating Menu

What did you expect to happen? What happened instead? What would you like to
see changed?

Focus wrap should work with the code provided on the website's demo code. The focus wrap does not work if you copy and paste the code in to another system and even the Carbon code pen demo does not work as expected.

What browser are you working in?

Chrome, Safari, Firefox

What version of the Carbon Design System are you using?

v10.4.1

What offering/product do you work on? Any pressing ship or release dates we
should be aware of?

Watson Health Design Pattern & Asset Library; Release date coming up is end of Q3

Steps to reproduce the issue

  1. Open the overflow menu and navigate to the last item in the list
  2. press Tab
  3. focus is moved outside of the menu and menu remains open

Please create a reduced test case in CodeSandbox

Doesn't work as expected
https://codepen.io/team/carbon/pen/PgNGop

Works once I add in extra incorrect element
https://codepen.io/elizabethsjudd/pen/mNmZdR
@elizabethsjudd elizabethsjudd mentioned this issue Jul 31, 2019
Closed
@asudoh
Copy link
Contributor

asudoh commented Aug 1, 2019

Hi 👋 thank you for reporting! One thing in consideration is adding focus-trap element in the end of floating menu body instead of letting application add one at end of <body>. The approach looks efficient especially with transclusion with template engines, like <slot></slot><input type="text"> where floating menu items will be put in <slot>.

@elizabethsjudd
Copy link
Contributor Author

elizabethsjudd commented Aug 1, 2019 •
edited
Loading

@asudoh we can't just add an empty input in to the code that doesn't do anything. This is extremely confusing for screen reader users as this element is not actually hidden for them (which is what allows it to be focusable and trigger the focus wrap.

Update:
now that I'm thinking about it... we could do something like <span tabindex="0"></span> to create a touch point that would not be confusing to screen reader users and could still be built in to the component HTML easily enough.

@asudoh
Copy link
Contributor

asudoh commented Aug 1, 2019

Good thoughts @elizabethsjudd - Another thing to bring up here is that such focus trap element won't be read as "focused" actually, given we change focus as soon as the trap element gets the focus.

@elizabethsjudd
Copy link
Contributor Author

@asudoh if you want to assign this issue to me I can work on making the updates. I know modals, overflow menu, and tooltips use this feature because they are ones WH currently has implemented. Are their other components that I should update as well?

@asudoh
Copy link
Contributor

asudoh commented Aug 1, 2019

If we can agree on .hbs change they we can go ahead - Thanks! You are right on the list of components. Also great if you can make the same change to the React codebase, too.

@elizabethsjudd
Copy link
Contributor Author

I can definitely make the change to the hbs templates. Unfortunately, I don't have time allocated to work on the react codebase at this time. We are trying to release our carbon 10 version of WH PAL and are blocked by these a11y updates to Carbon. My team has allowed me to work on the Carbon Vanilla work since this is blocking us from helping our users and moving forward with our projects especially since the Carbon core team has deprioritized most Carbon Vanilla work for the time being.

@elizabethsjudd
Copy link
Contributor Author

@asudoh do you want to assign this issue to me?

@asudoh
Copy link
Contributor

asudoh commented Aug 2, 2019

I did, but I'd say, don't wait for assignment - Carbon is open source!

@elizabethsjudd
Copy link
Contributor Author

@asudoh Thanks for assigning it to me! It's for my own team's processes that I request the issues be assigned to me before working on them.

@elizabethsjudd elizabethsjudd mentioned this issue Aug 5, 2019
Merged
@hixio-mh hixio-mh mentioned this issue Jan 22, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@elizabethsjudd elizabethsjudd

Labels
type: bug 🐛
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(floating-menu): add focus wrap element to components elizabethsjudd/carbon
2 participants
@asudoh @elizabethsjudd