Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

v4 - Remove confusing unnecessary id/aria-labelledby for dropdown menus - #36491

#36491
Merged
patrickhlauke merged 1 commit into
v4-devtwbs/bootstrap:v4-devfrom
patrickhlauke-remove-optional-aria-labelledby-v4-porttwbs/bootstrap:patrickhlauke-remove-optional-aria-labelledby-v4-portCopy head branch name to clipboard
Jun 3, 2022
Merged

v4 - Remove confusing unnecessary id/aria-labelledby for dropdown menus#36491
patrickhlauke merged 1 commit into
v4-devtwbs/bootstrap:v4-devfrom
patrickhlauke-remove-optional-aria-labelledby-v4-porttwbs/bootstrap:patrickhlauke-remove-optional-aria-labelledby-v4-portCopy head branch name to clipboard

Conversation

@patrickhlauke

Copy link
Copy Markdown
Member

v4 port of #36487

@patrickhlauke
patrickhlauke requested a review from a team as a code owner June 3, 2022 11:05
@patrickhlauke
patrickhlauke requested a review from mdo June 3, 2022 11:05
@patrickhlauke patrickhlauke changed the title Remove confusing unnecessary id/aria-labelledby for dropdown menus v4 - Remove confusing unnecessary id/aria-labelledby for dropdown menus Jun 3, 2022

@julien-deramond julien-deramond left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've maybe spotted one we forgot in the original PR (and here). In site/layouts/partials/docs-navbar.html:

<div class="dropdown-menu dropdown-menu-md-right" aria-labelledby="bd-versions">

@patrickhlauke

Copy link
Copy Markdown
Member Author

@julien-deramond yeah, i saw that one, but that's our own use of it on the docs page. i still think it's a nice-to-have little addition - doesn't make a huge accessibility difference, but gives a bit more context. just not something important enough to warrant inclusion in every example we provide (where it then confused people thinking they needed to do it for it to work)

@julien-deramond
julien-deramond self-requested a review June 3, 2022 16:14
@patrickhlauke

Copy link
Copy Markdown
Member Author

@julien-deramond having said that, I just checked how it announces in chrome/nvda ... and it didn't announce the label for the list at all for some reason. so yeah, might as well remove it...

@patrickhlauke
patrickhlauke merged commit b4c994d into v4-dev Jun 3, 2022
@patrickhlauke
patrickhlauke deleted the patrickhlauke-remove-optional-aria-labelledby-v4-port branch June 3, 2022 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.