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

Non-unique ids for DOM elements #1106

Closed
artlowel opened this issue Apr 16, 2021 · 2 comments
Closed

Non-unique ids for DOM elements #1106

artlowel opened this issue Apr 16, 2021 · 2 comments
Assignees
Milestone

Comments

@artlowel
Copy link
Member

Describe the bug
Since we merged #1057, a number of warnings about non-unique ids have shown up in the dev console:

Screen Shot 2021-04-16 at 16 58 12

The problematic elements are the username and password inputs for the login form and the search input in the header. They're likely duplicated to be rendered differently for small and large screens. Possible solutions could be to just remove the IDs, angular likely doesn't need them. Or if that's an accessibility problem: only render each version if that grid breakpoint is active according to hostWindowService

To Reproduce
Steps to reproduce the behavior:

  1. Open the homepage
  2. Look at the browser dev tools

Expected behavior
We shouldn't duplicate HTML IDs

Related work
#1057

@artlowel artlowel added bug low priority needs triage New issue needs triage and/or scheduling labels Apr 16, 2021
@tdonohue tdonohue added accessibility and removed needs triage New issue needs triage and/or scheduling labels Apr 16, 2021
@tdonohue
Copy link
Member

I agree, and this sounds like an accessibility issue that we should fix prior to 7.0. That said, I'm bumping this up to medium priority, as I know duplicate IDs cause issues in screen readers and similar... plus, I know accessibility is a priority for Steering.

Some fields do require an ID, if they are referenced in an ARIA attribute (by their ID) or by a label tag (again by their ID). So, if these fields require an ID they must be unique IDs. If they do not require an ID, then we could just remove the IDs altogether. I've not checked which scenario applies here.

As a sidenote, Deque (the firm we are doing accessibility testing with during Testathon) has a Chrome plugin which may be helpful here to analyze the final HTML. https://chrome.google.com/webstore/detail/axe-devtools-web-accessib/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US

@atarix83 : I'm assigning this to you (or your team) to analyze further & report back on.

@tdonohue tdonohue added this to the 7.0 milestone Apr 16, 2021
@tdonohue tdonohue added e/0 Estimate in hours and removed Estimate TBD labels Jul 28, 2021
@tdonohue
Copy link
Member

This appears to be duplicated by #1167 (see first bullet listed there) and it was fixed by #1252

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

No branches or pull requests

3 participants