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

♿ [#2376] Add aria-expanded for desktop navigation menus #1246

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

jiromaykin
Copy link
Contributor

@jiromaykin jiromaykin commented Jun 10, 2024

Taiga: https://taiga.maykinmedia.nl/project/open-inwoner/task/2376

aria-expanded should be added to the element that controls the expanded state of other elements.

Due to Apple's conscious decision in 2008 to not set focus on clicked buttons, this still behaves slightly different in Safari (opening 1 menu does not close the other there).

@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.20%. Comparing base (dedbd14) to head (6b08ca2).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1246   +/-   ##
========================================
  Coverage    95.20%   95.20%           
========================================
  Files          978      978           
  Lines        35603    35603           
========================================
  Hits         33896    33896           
  Misses        1707     1707           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jiromaykin jiromaykin force-pushed the issue/2376-aria-expanded-desktop-navs branch from 892da9b to dd1b3c7 Compare June 11, 2024 12:14
@jiromaykin jiromaykin added the wip Work in progress label Jun 11, 2024
@jiromaykin jiromaykin force-pushed the issue/2376-aria-expanded-desktop-navs branch 2 times, most recently from 39d393e to 6b08ca2 Compare June 17, 2024 13:14
@jiromaykin jiromaykin added the Accessibility Improving accessibility label Jun 18, 2024
@jiromaykin jiromaykin force-pushed the issue/2376-aria-expanded-desktop-navs branch from 7eb41ea to 57d29d8 Compare June 20, 2024 20:58
@jiromaykin jiromaykin marked this pull request as ready for review June 20, 2024 20:59
@jiromaykin jiromaykin removed the wip Work in progress label Jun 20, 2024
@jiromaykin jiromaykin requested a review from pi-sigma June 20, 2024 21:08
Copy link
Contributor

@pi-sigma pi-sigma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we only need this for the "Onderwerpen" button? The "Welkom" button currently doesn't have an aria-expanded attribute

@jiromaykin
Copy link
Contributor Author

Do we only need this for the "Onderwerpen" button? The "Welkom" button currently doesn't have an aria-expanded attribute

? But it does. And yes, they both need it.

Copy link
Contributor

@pi-sigma pi-sigma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, was looking in the wrong place for the "Welkom" button. Works fine (though only tested on FireFox and Chrome).

@alextreme alextreme merged commit 451a4e6 into develop Jun 25, 2024
16 checks passed
@alextreme alextreme deleted the issue/2376-aria-expanded-desktop-navs branch June 25, 2024 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility Improving accessibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants