Skip to content

Ada 233#58

Merged
yzhoubk merged 3 commits intomainfrom
ADA-233
Jun 6, 2025
Merged

Ada 233#58
yzhoubk merged 3 commits intomainfrom
ADA-233

Conversation

@yzhoubk
Copy link
Copy Markdown
Contributor

@yzhoubk yzhoubk commented Jun 6, 2025

ADA-233: Ensure landmarks are unique

@yzhoubk yzhoubk requested a review from awilfox June 6, 2025 16:42
@yzhoubk yzhoubk assigned yzhoubk and unassigned awilfox and yzhoubk Jun 6, 2025
Copy link
Copy Markdown
Member

@awilfox awilfox left a comment

Choose a reason for hiding this comment

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

I don't think this is exactly correct.

  • Firstly, screen readers typically announce the role, so the NAV element would announce as "Main navigation navigation".
  • However, I don't think an aria-label should be needed on the NAV, because it should be the only element with a navigation role. The search bar does not serve a navigation role; if anything it serves a search role. The Blacklight search form has the search role defined on it correctly, so I do not think we should be setting the role on this element.

I think the best, and simplest, way forward is to simply remove the role="navigation" from the search bar's containing DIV. No other changes should be needed as far as I can tell. Tagging @jxloesberg for his input.

@jxloesberg
Copy link
Copy Markdown

I agree with @awilfox. <nav> is sufficient here, and so the role attribute redundant, and generates confusion. I also recommend simply removing it. No other changes are required.

@yzhoubk
Copy link
Copy Markdown
Contributor Author

yzhoubk commented Jun 6, 2025

Thanks for the suggestions!

@yzhoubk yzhoubk merged commit edb411a into main Jun 6, 2025
2 checks passed
@yzhoubk yzhoubk deleted the ADA-233 branch June 6, 2025 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants