UI: a11y sidebranch - labels #24480
UI: a11y sidebranch - labels #24480
Conversation
github-actions
bot
added
the
hashicorp-contributed-pr
| <Hds::Button | ||
| class="hds-side-nav__icon-button" | ||
| {{on "click" (action "fullscreen")}} | ||
| title={{if this.isFullscreen "minimize" "maximize"}} |
There was a problem hiding this comment.
Tooltip trigger didn't have valid label associated, so I removed it. With a title on the icon button sighted users can still see the text
|
Build Results: |
hashishaw
force-pushed
the
ui/a11y-testing/labels
branch
from
115dde8 to
1d4e25b
Compare
…r was showing up twice. Instead pass @ariaLabel explicitly to <Select>
| >{{this.key.value}}</textarea> | ||
| </div> | ||
| <p class="help has-text-grey"> | ||
| <p class="help has-text-grey" id={{concat "pgpFileTextarea-" this.elementId}}> |
There was a problem hiding this comment.
| <p class="help has-text-grey" id={{concat "pgpFileTextarea-" this.elementId}}> | |
| <p class="help has-text-grey" id="pgpFileTextarea-{{this.elementId}}"> |
| data-test-pgp-file-textarea={{true}} | ||
| aria-labelledby={{concat "pgpFileTextarea-" this.elementId}} |
There was a problem hiding this comment.
Not necessary to update, just suggesting
| data-test-pgp-file-textarea={{true}} | |
| aria-labelledby={{concat "pgpFileTextarea-" this.elementId}} | |
| data-test-pgp-file-textarea | |
| aria-labelledby="pgpFileTextarea-{{this.elementId}}" |
There was a problem hiding this comment.
Ah - I see you just copy/pasted from above! Disregard!
| @@ -38,9 +38,10 @@ | |||
| class="textarea" | |||
| oninput={{action "updateData"}} | |||
| data-test-pgp-file-textarea={{true}} | |||
| aria-labelledby={{concat "pgpFileTextarea-" this.elementId}} | |||
There was a problem hiding this comment.
Pointing it out for the future but we'll need a different solution for a unique id when this component is refactored to glimmer.
There was a problem hiding this comment.
Good note! I've used guidFor in other places when I need the element ID on glimmer components: https://ember-learn.github.io/ember-octane-vs-classic-cheat-sheet/#component-templates__element-id
| @@ -6,7 +6,7 @@ | |||
| {{#unless @uploadOnly}} | |||
| <div class="level is-mobile"> | |||
| <div class="level-left"> | |||
| <label for="input-{{this.elementId}}" class="has-text-weight-semibold" data-test-text-file-label> | |||
| <label id="text-file-input-label" class="has-text-weight-semibold" data-test-text-file-label> | |||
There was a problem hiding this comment.
I think we will eventually need a way to easily add a unique value to things like id's since the component could be used multiple times on the same page. Maybe a helper that uses guidFor or something along those lines.
There was a problem hiding this comment.
https://rfcs.emberjs.com/id/0659-unique-id-helper/
if it can be of help, this is a nice syntax/helper to add IDs directly in the template, we've used in a few places in HDS (to be checked if the Ember version you're using in Vault supports it)
There was a problem hiding this comment.
Good point, I do want the ID's to be unique. Now that the other tests are somewhat stable I'll put the elementID back and check whether it's still failing
There was a problem hiding this comment.
Update: it was originally failing in one test, when uploadOnly is passed which does not render the label. In other places I tried to make the input ID's deterministic so that if we do have a label outside of the input, we can add for="the-id" to the label and it connects it, but when we use random ID's that's not possible. I think this is something to think about for our form refresh, but for now I just added a screenreader-only label when uploadOnly is true.
hashishaw
merged commit 0856125
into
ui/VAULT-22618/add-a11y-testing
This PR adds labels to tags that require them (
<nav>,<input>, etc). The changes in this PR are based on failures from the Integration tests only.Unfortunately, a couple 3rd party components that we use (CodeMirror and ember power select to be specific) have violations that are causing widespread failures. But, I tried to address all the instances that we can control.