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

♿ a11y(bal-nav): button overlays not reachable via keyboard #1415

Open
1 task done
carolabes opened this issue Jun 11, 2024 · 15 comments · Fixed by #1438 or #1456 · May be fixed by #1479
Open
1 task done

♿ a11y(bal-nav): button overlays not reachable via keyboard #1415

carolabes opened this issue Jun 11, 2024 · 15 comments · Fixed by #1438 or #1456 · May be fixed by #1479
Assignees
Labels
🐛 bug Something isn't working

Comments

@carolabes
Copy link

Description of this issue

  • Button overlays are not reachable via keyboard
meta-nav-popup

How to fix it

  • Use aria-haspopup for buttons with overlays
  • Don't use aria-haspopup for button with link

Additional Information

Level A
https://www.w3.org/WAI/WCAG21/Understanding/keyboard.html

aria-haspopup: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-haspopup

Code of Conduct

  • I agree to follow this project's Code of Conduct
@carolabes carolabes added the ✨ feature New feature or request label Jun 11, 2024
@hirsch88 hirsch88 added 🐛 bug Something isn't working and removed ✨ feature New feature or request labels Aug 8, 2024
@hirsch88 hirsch88 changed the title ♿ a11y(bal-nav): button overlays not reachable via keyboard ♿ a11y(bal-nav): button overlays not reachable via keyboard Aug 8, 2024
@mmarinkov mmarinkov self-assigned this Aug 23, 2024
@mmarinkov
Copy link
Contributor

/cib

Copy link
Contributor

Branch fix/issue-1415 created!

@mmarinkov
Copy link
Contributor

Hi @carolabes,
The button with the Kundenservice number was reachable, the close button not.
I assume this was the reason for opening the case or am I wrong?

@carolabes
Copy link
Author

Hi @mmarinkov,
I rechecked this issue on live page and DS. You are right, the phone number button is reachable. I didn't realize that because the order is wrong. The pop ups are placed after the main nav. On the live page I can only reach them if I tab through the entire navigation.
I'm not sure if there is already a ticket for the order. But yes, the close button should also be accessible.

@mmarinkov
Copy link
Contributor

Hi @carolabes
I would say that #1423 is related to the order issue when navigation via tab. In #1423 we have to prevent access to the background of a modal/pop via tab. So by solving #1423 we should also solve the order issue in this case.

In this case we will work on making the "close button" reachable via keyboard tab.

@mmarinkov
Copy link
Contributor

would need to change tabindex to 0 but this logic would overwrite the one introduced by Gery (5377a64, fix(select): value with comma, click events, 07.09.2022 at 21:57)

Next step: clarify what would break when changing the tabindex

@hirsch88 hirsch88 assigned hirsch88 and unassigned mmarinkov Sep 6, 2024
hirsch88 added a commit that referenced this issue Sep 6, 2024
* Create PR for #1415

* fix(close): make close button focusable by changing tabindex from -1 to 0

* fix(close): make close button focusable by changing tabindex from -1 to 0

* chore: add changeset

* chore: add changeset

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Marinkov, Magdalena <[email protected]>
Co-authored-by: Gery Hirschfeld <[email protected]>
@mmarinkov mmarinkov reopened this Sep 12, 2024
@mmarinkov
Copy link
Contributor

/cib

Copy link
Contributor

Branch fix/issue-1415 created!

github-actions bot added a commit that referenced this issue Sep 12, 2024
mmarinkov pushed a commit that referenced this issue Sep 12, 2024
@mmarinkov mmarinkov assigned mmarinkov and unassigned hirsch88 Sep 12, 2024
hirsch88 pushed a commit that referenced this issue Sep 13, 2024
* Create PR for #1415

* Create PR for #1415

* fix(close): remove inherited tab index attribute

* fix(close): added changeset

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Marinkov, Magdalena <[email protected]>
@GinaBiondo-aperto
Copy link
Collaborator

Hey @hirsch88 :) the issue also mentioned an aria-haspopup prop. Was this added as well or should this be passed with the json just like the aria labels?

@mmarinkov
Copy link
Contributor

Hey @GinaBiondo-aperto,
the aria-haspopup prop has not been set on the button with link as initially requested in this task.

In our opinion the issue seems to be resolved or do you see something, which we missed?

@GinaBiondo-aperto
Copy link
Collaborator

Hey Hey @mmarinkov :) Not sure if I get your point here. Help please :D The aria-haspopup is not set at all. Neither the buttons with link nor without. However it is needed for the buttons in the meta nav that open popups.

@mmarinkov
Copy link
Contributor

We have to set aria-haspopup prop on the button components expect the ones with color="link"

@mmarinkov
Copy link
Contributor

/cib

Copy link
Contributor

Branch fix/issue-1415 created!

github-actions bot added a commit that referenced this issue Oct 10, 2024
@hirsch88 hirsch88 added this to the ♿️ Accessibility milestone Nov 8, 2024
@hirsch88 hirsch88 removed their assignment Dec 5, 2024
@mmarinkov mmarinkov assigned m4rc0z and unassigned mmarinkov Dec 13, 2024
@mmarinkov
Copy link
Contributor

we have to define the aria-haspopup on the popup compoment (not popover)

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