joshkimux commented 3 years ago

508-defect-2

⚠️ RECOMMENDATION MAY BE OUTDATED WITH INTRODUCTION OF NEW SUBTASK PATTERN ⚠️

Feedback framework

❗️ Must for if the feedback must be applied
⚠️ Should if the feedback is best practice
✔️ Consider for suggestions/enhancements

Definition of done

Review and acknowledge feedback.
Fix and/or document decisions made.
Accessibility specialist will close ticket after reviewing documented decisions / validating fix.

Point of Contact

VFS Point of Contact: Josh

User Story or Problem Statement

As a screen reader user, I want to know which document I am editing or deleting on the supporting evidence page without having to manually find the document title.

Details

If a screen reader user were to tab through the supporting evidence list loop pattern after uploading multiple documents, they would hear something like this on each tab stop:

Upload your treatment... (legend), list with 3 items (entering a list), document type (label), STR Dental Photocopy (select box that has focus)
Delete file button

what file am I deleting?

Document type (label), STR Dental Photocopy (select box that has focus)

which file did I select STR Dental Photocopy for?

Delete file button
Document type (label), STR Dental Photocopy (select box that has focus)

which file is this?

Delete file button

This is particularly troubling for users with cognitive disabilities as well.

Acceptance Criteria

[ ] Labels and buttons have descriptive labels

Environment

Operating System: any
Browser: any
Screenreading device: any
Server destination: production

Steps to Recreate

Go to the supporting evidence page
Upload 2+ documents
Tab through and confirm there are no descriptive labels

Possible Solutions

As these are all collectively already nested within a fieldset, it would be difficult to approach this from a pure html standpoint by nesting each individual document within its own nested fieldset.

Aria-describedby

We could take advantage of the existing strong elements that include the document's title and use it as a way to describe their respective labels and buttons by using aria-describedby.

For example (simplified with only relevant items)

Before

<li>
  <strong>file name 1</strong>
    <label>Document type</label>
    <select id="" name="">
      <option value=""></option>
      <option value="L450">STR - Dental - Photocopy</option>
      <option value="L451">STR - Medical - Photocopy</option>
    </select>
  <button>Delete file</button>
</li>
<li>
  <strong>file name 2</strong>
    <label>Document type<span class="schemaform-required-span">(*Required)</span></label>
    <select id="" name="">
      <option value=""></option>
      <option value="L450">STR - Dental - Photocopy</option>
      <option value="L451">STR - Medical - Photocopy</option>
    </select>
  <button>Delete file</button>
</li>

After

<li>
+  <strong id="file-name-1">file name 1</strong>
      <label>Document type</label>
+    <select aria-describedby="file-name-1">
      <option value=""></option>
      <option value="L450">STR - Dental - Photocopy</option>
      <option value="L451">STR - Medical - Photocopy</option>
    </select>
+  <button aria-describedby="file-name-1">Delete file</button>
</li>
<li>
+  <strong id="file-name-2">file name 2</strong>
      <label aria-describedby="file-name-2">Document type<span class="schemaform-required-span">(*Required)</span></label>
+    <select aria-describedby="file-name-2">
      <option></option>
      <option value="L450">STR - Dental - Photocopy</option>
      <option value="L451">STR - Medical - Photocopy</option>
    </select>
+  <button aria-describedby="file-name-2">Delete file</button>
</li>

Aria-labelledby

We could also use aria-labelledby, which may be more appropriate as it will replace the accessible name. This could add together the file name and the label. For example:

Before

<li>
  <strong>file name 1</strong>
    <label>Document type</label>
    <select id="" name="">
      <option value=""></option>
      <option value="L450">STR - Dental - Photocopy</option>
      <option value="L451">STR - Medical - Photocopy</option>
    </select>
  <button>Delete file</button>
</li>
<li>
  <strong>file name 2</strong>
    <label>Document type<span class="schemaform-required-span">(*Required)</span></label>
    <select id="" name="">
      <option value=""></option>
      <option value="L450">STR - Dental - Photocopy</option>
      <option value="L451">STR - Medical - Photocopy</option>
    </select>
  <button>Delete file</button>
</li>

After

<li>
+  <strong id="file-name-1">file name 1</strong>
+    <label id="label-1">Document type</label>
+    <select aria-labelledby="file-name-1 label-1">
      <option value=""></option>
      <option value="L450">STR - Dental - Photocopy</option>
      <option value="L451">STR - Medical - Photocopy</option>
    </select>
+  <button aria-describedby="file-name-1">Delete file</button>
</li>
<li>
+  <strong id="file-name-2">file name 2</strong>
+    <label id="label-2">Document type<span class="schemaform-required-span">(*Required)</span></label>
+    <select aria-labelledby="file-name-2 label-2">
      <option></option>
      <option value="L450">STR - Dental - Photocopy</option>
      <option value="L451">STR - Medical - Photocopy</option>
    </select>
+  <button aria-describedby="file-name-2">Delete file</button>
</li>

Would love to get your eye on this recommendation @1Copenut as this one is a bit tricky. Has this emerged elsewhere before, and if so, how have we handled it?

Screenshots or Trace Logs

This also exists on the review answers page where the user can tab through 3 buttons which all say "delete file button" delete file repeats

Mottie commented 3 years ago

I haven't worked on this yet, but when you get a chance, can you try this with a password protected PDF? You can download example files from this comment - https://github.com/department-of-veterans-affairs/va.gov-team/issues/14011#issuecomment-703991496 - it adds a password input with "Add password" button.

joshkimux commented 3 years ago

I'm experiencing some bugs where focus will always reroute to the add password field 😕

https://user-images.githubusercontent.com/14154792/114439207-bdfff500-9b96-11eb-9047-28ce880a2865.mov

Also, the error for an incorrect password isn't being announced 😦

https://user-images.githubusercontent.com/14154792/114439901-804f9c00-9b97-11eb-9c65-f4a41bbbf9db.mov

Mottie commented 3 years ago

Fix merged into master, it should be available in staging soon. Please verify at your leisure 😺

joshkimux commented 3 years ago

@Mottie Thanks for this! I just started and testing and realized this is more verbose than I anticipated due to the nested fieldsets (something I should have absolutely considered from the beginning).

I'm going to see if I can iterate on this a bit with @AngelaFowler82 on codepen to see if there would be ways to find a better balance. I'd love to get your help there too from a technical perspective.

joshkimux commented 2 years ago

@Mottie just letting you know I haven't forgotten this and have whipped up a codepen to test it. I came across a snag where the list loop for military service history can conflict with uploaded documents.

Specifically, how do we provide unique labels without the verbosity that can scale across all instances of list loop implementation?

For example:

a Veteran could have multiple service periods for the army
a user could upload two documents with the same name

Questions I'm working through:

How might we provide unique labels in these instances without complicated logic or changes?
How might we reduce verbosity without sacrificing specificity?
Is this something we can address through remediation of code, or are deeper design changes necessary?

What I've tried/considering:

Using aria-label with aria-describedby to add extra details to distinguish labels apart. For example, the aria-label could contain a short label e.g. "Update army" while the aria-describedby could provide additional differentiators e.g. "Service period from 2010 to 2016." Problem: I don't think this scales to other instances of the list loop and would be a one off for service history.
Using headings as legends
Collaborating with Ryan, Angela, and Martha on an updated list loop pattern

joshkimux commented 2 years ago

@caw310 👋 Quick question for you-- is the list loop pattern currently being worked on in the design or forms system? If so, would there be anyway for us to link this ticket up as a case to solve for?

caw310 commented 2 years ago

@joshkimux , There is a list loop pattern that is deployed. Here is a link to that pattern. https://design.va.gov/patterns/forms/list-and-loop

Adding Katy in case she has more insights on this.

joshkimux commented 2 years ago

@caw310 Thank you! I think this is a bit more of a complex case as it involves a file upload instead of an input field... is that too much of an edge case for the design system team to consider in the future? @k80bowman

k80bowman commented 2 years ago

We don't have any plans to revisit the list-loop pattern currently, but we could potentially look into this in the future. I'm not sure how much of an edge case it is without an audit to see how often this type of pattern is being implemented.

joshkimux commented 2 years ago

This might be used with 10-10ez as well? @Dene-human could you help confirm that? Otherwise, if it's benefits specific we can see what we can do about it within our team.

Dene-human commented 2 years ago

Hey! We might have a couple of overlaps.

On the insurance detail we have an array of items- and we are also waiting on a bug fix (ticket). Another note, our goal is to split this page, but we can't currently have an array of items on its own page because of how the Review page handles it.
In the Military History section we upload a doc, but just a single file (not list loop)

joshkimux commented 1 year ago

@RakshindaAslam I see you've added the disability-experience label to this! Just as a heads up for context, I strongly recommend working with the design system team, as this issue has cropped up on multiple teams.

RakshindaAslam commented 1 year ago

Thanks @joshkimux! cc @lydiahooper

joshkimux commented 1 year ago

Updated assignees, added design system label, and added note to ticket on the subtask pattern

mengA6 commented 6 months ago

Is this still relevant, or has it been resolved at the design system level?

Mottie commented 6 months ago

I think it's still relevant:

The select is set using uiSchema:

If not recently updated, it's likely using a React select widget. These weren't set up to add an aria-describedby.
- If it was recently updated, then it's using a va-select. This web component doesn't have the ability to add a dynamically set (with file name) aria-describedby

The delete button has been updated, and does include the file name within the aria-label, so this one should no longer be an issue.

Here's an example of our team's Supplemental Claim form:

<li id="root_additionalDocuments_file_0" class="va-growable-background">
  <p>You’re adding this evidence:</p>
  <strong
    id="root_additionalDocuments_file_name_0"
    class="dd-privacy-hidden"
    data-dd-action-name="file name"
  >
    Test.pdf
  </strong>
  <div class="schemaform-file-attachment review">
    <va-select
      class="rjsf-web-component-field hydrated"
      label="Document type"
      name="root_additionalDocuments_0_attachmentId"
      required="true"
      uswds
      value="L015"
      <!-- we could add `message-aria-describedby` here, but it'd be a fixed message -->
    >
      <option value="L015">Buddy/Lay Statement</option>
      <!-- ... -->
    </va-select>
  </div>

  <div class="vads-u-margin-top--2">
    <va-button
      secondary
      class="delete-upload vads-u-width--auto hydrated"
      label="Delete Test.pdf" <!-- defines aria-label -->
      text="Delete file"
      uswds
    />
  </div>
</li>

Mottie commented 6 months ago

Hmm, actually... I think I can update the FileField to pass in the file name and then we include a message-aria-describedby and that should fix it!

Mottie commented 6 months ago

I've got a PR open to fix this. Once merged, and the component library has been updated, you can use both of these options:

attachmentSchema: ({ fileId, index, fileName }) => ({
  'ui:title': 'Document type',
  'ui:options': {
    // messageAriaDescribedby utilized by web components
    // This already works for `va-text-input`, but library update needed
    // to support `va-select`
    messageAriaDescribedby: `Select a document type for ${fileName}`,

    // widget props for default form library widgets (non-web components)
    widgetProps: {
      'aria-describedby': fileId,
      'data-index': index,
    },
  },
}),

Mottie commented 6 months ago

Use the above pattern to fix the select and/or document name input. Then we can close this ticket as complete.

Mottie commented 5 months ago

Following up here. The component library has been updated and I checked our Supplemental Claim form upload page. The document type select now includes a descriptive label. Our Notice of Disagreement form does not ask for a document type.

va-select message-aria-describedby now includes a span inside the shadow dom with the descriptive message

@saderagsdale We can remove our team from this ticket. I just checked 526, and the upload page is not yet using a va-select web component, but it does include an aria-describedby

department-of-veterans-affairs / va.gov-team

508-defect-2 [COGNITION, SCREEN READER]: List loop buttons and fields SHOULD have descriptive labels #19696

508-defect-2

Feedback framework

Definition of done

Point of Contact

User Story or Problem Statement

Details

Acceptance Criteria

Environment

Steps to Recreate

Possible Solutions

Aria-describedby

Before

After

Aria-labelledby

Before

After

Screenshots or Trace Logs