search

department-of-veterans-affairs / vets-design-system-documentation

Repository for design.va.gov website
https://design.va.gov
36 stars 55 forks source link

Segmented and non-segmented progress bar components throw an axe (accessibility plugin) violation in the newest ruleset #357

Closed 1Copenut closed 3 years ago

1Copenut commented 3 years ago

Bug Report

What happened

The newest version of the Deque axe browser plugin flags our Segmented Progressbar component because it doesn't have an accessible label. This will cause our CI/CD pipeline to break when we upgrade the axe-core library. That work is planned for after the holiday.

Pinging @joshkimux for visibility. He's been looking closely at the Progressbar and has some really good insights.

This issue affects our multi-step forms and will block bumping the axe-core library because of the potential for CI/CD build breaks:

What I expected to happen

Reproducing

Steps to reproduce:

  1. Open any multi-step form that users our <Progressbar /> component
  2. Open Chrome / Firefox dev tools
  3. Run the axe browser plugin (assumes the plugin has been added to your browser)
  4. Verify the serious violation "ARIA progressbar nodes must have an accessible name" appears in your dev tools

Potential Code Updates

I took a look at how we might update the markup for our Progress bar. Put together a Codepen and the snippet below. JAWS will only read the aria-valuetext while NVDA and VoiceOver will read the aria-label, then the aria-valuetext in preliminary reviews.

<div>
  <div
!   Consider using a "title" attribute instead, must retest first
+   aria-label="Form 22-1990, education benefits"
    aria-valuemax="8"
    aria-valuemin="0"
    aria-valuenow="1"
+   aria-valuetext="Step 1 of 8: Applicant information"
    class="progress-bar-segmented"
    role="progressbar"
    tabindex="0"
  >
    <div class="progress-segment progress-segment-complete"></div>
    <div class="progress-segment"></div>
    <div class="progress-segment"></div>
    <div class="progress-segment"></div>
    <div class="progress-segment"></div>
    <div class="progress-segment"></div>
    <div class="progress-segment"></div>
    <div class="progress-segment"></div>
  </div>
  <div class="schemaform-chapter-progress">
    <div
-     aria-valuemax="8"
-     aria-valuemin="1"
-     aria-valuenow="1"
-     aria-valuetext="Step 1 of 8: Applicant information"
      class="nav-header nav-header-schemaform"
    >
      <h2 class="vads-u-font-size--h4" id="nav-form-header" tabindex="-1">
        Step 1 of 8: Applicant information
      </h2>
    </div>
  </div>
</div>

Screenshot

Screen Shot 2020-12-11 at 12 54 04 PM

caw310 commented 3 years ago

@cvalarida @bkjohnson , can you look into this bug next week?

JoeTice commented 3 years ago

@caw310 @cvalarida @bkjohnson - Have you had an opportunity to look into this? We are trying to determine how we should proceed. Thanks in advance for any info/insights you can provide!

caw310 commented 3 years ago

@JoeTice , will try to discuss this during our planning on weds and get back to you.

cvalarida commented 3 years ago

Action items for the SegmetedProgressBar

Action items for the forms library to use the ProgressBar

In FormNav:

The result will look like the following:

<SegmentedProgressBar
  total={chapters.length}
  current={current}
  label={`${formConfig.subTitle}, ${formConfig.title}`}
  ariaValueText={stepText} />

Action items for the ProgressBar

bkjohnson commented 3 years ago

Showing the error in Storybook (the same appears for the non-segmented version):

image.png

bkjohnson commented 3 years ago

Here is how our ProgressBars are used in vets-website:

rg "<(.)*ProgressBar" src/ -g "!test"

src/applications/claims-status/components/UploadStatus.jsx
15:        <ProgressBar percent={progress * 100} />

src/applications/letters/containers/DownloadLetters.jsx
22:      <SegmentedProgressBar total={chapters.length} current={currentStep} />

src/applications/vre/28-1900/orientation/StepComponent.jsx
18:        <SegmentedProgressBar current={data.number} total={stepTotal} />

src/platform/forms-system/src/js/components/FormNav.jsx
83:      <SegmentedProgressBar total={chapters.length} current={current} />

src/platform/forms-system/src/js/fields/FileField.jsx
287:                      <ProgressBar percent={this.state.progress} />
1Copenut commented 3 years ago

Thank you @bkjohnson. I'll roll this in with the work I'm doing on the other side for bumping axe-core. Will have answers for you this week hopefully. Lots of moving parts, but we're getting there.

1Copenut commented 3 years ago

@bkjohnson I added text for each of the five instances, in the same order as your regex search returned them. LMK if you have any questions or these text blocks can't be easily assembled.

Upload Status text

<ProgressBar /> component

Step Component text

<SegmentedProgressBar /> component

Form Nav text

<SegmentedProgressBar /> component

File Field text

<ProgressBar /> component

bkjohnson commented 3 years ago

The Design System team has discussed this, and due to the expanding scope we have decided to pause implementation and hand it off to @JoeTice.

1Copenut commented 3 years ago

This issue has been resolved with PR #28 in Darius' comment above. Moving to close.

joshkimux commented 3 years ago

@1Copenut Issue 21138 created by Angela during her HLR audit may be relevant to this and may require a new ticket

1Copenut commented 3 years ago

@joshkimux I think it will be a new ticket for the design system team, yes. Reading Angela's ticket, I understood that the progressbar shouldn't have a tabindex=0" because that made it a focusable element.

Would you mind opening a new ticket on the https://github.com/department-of-veterans-affairs/vets-design-system-documentation/ repo?

joshkimux commented 3 years ago

On it!