[Accessibility][Storybook] Add aria-labels to the multiple item progress bar + enforce aria-label in progressbar.item

[kendallgassner]

Closes https://github.com/github/accessibility-audits/issues/10134

In this PR I add an aria-label to each role='progressbar'. I also have updated the ProgressBar.item props to enforce that aria-labels are always added

Changelog

New

Changed

Removed

Rollout strategy

• Patch release
• Minor release
• Major release; if selected, include a written rollout or migration plan
• None; if selected, include a brief description as to why

I am updating a storybook story.

Testing & Reviewing

Tested with a screen reader.

Merge checklist

• Added/updated tests
• Added/updated documentation
• Added/updated previews (Storybook)
• Changes are SSR compatible
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge
• (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

[changeset-bot]

🦋 Changeset detected

Latest commit: f02fd75

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	104.49 KB (-0.05% 🔽)
packages/react/dist/browser.umd.js	104.85 KB (-0.02% 🔽)

[khiga8]

Makes sense!

• Do you think that aria-label should be required on ProgressBar.Item?
• I see this was an Axe violation. I thought all Primer React storybook examples have Axe checks...maybe this doesn't'? do you know why this wasn't flagged?

[kendallgassner]

Do you think that aria-label should be required on ProgressBar.Item?

@khiga8 I like this, good catch.

I see this was an Axe violation. I thought all Primer React storybook examples have Axe checks...maybe this doesn't'? do you know why this wasn't flagged?

Its failing on storybook prod. 🤔 Do we not have an a CI check?

[khiga8]

Do you think that aria-label should be required on ProgressBar.Item?

@khiga8 I like this, good catch.

I see this was an Axe violation. I thought all Primer React storybook examples have Axe checks...maybe this doesn't'? do you know why this wasn't flagged?

Its failing on storybook prod. 🤔 Do we not have an a CI check?

There's some kind of CI axe checks running, but I'm not sure if it's running on this story...

[github-actions]

👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

[kendallgassner]

@khiga8 I'll update the testing in this PR. There are 30 failures, so I'll have to play around with it a bit.

[kendallgassner]

@TylerJDev what do you think should this be minor changeset? Since the props now enforce developers add aria-labels?

[TylerJDev]

@kendallgassner, let's see how it runs in Dotcom's CI! I'll run an integration test PR for us. I feel like minor is the right call, but we'll see if it's a breaking change or not in the test PR 🤔

[primer-integration]

👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/358710

[primer-integration]

🔴 golden-jobs completed with status failure.

[kendallgassner]

3 ish changes needed for 🔴 golden-jobs to pass. I will make a PR in dotcom.

[hectahertz]

@TylerJDev what do you think should this be minor changeset? Since the props now enforce developers add aria-labels?

If we are making the prop required it should be a breaking change; even if it's not breaking dotcom, it might be breaking for other users of Primer React.

[kendallgassner]

Now switching this to a 'console.warn' so I think we can move back to patch.

[khiga8]

Nice! Thanks for following up and investigating if/how to require the aria-label prop! Good call pivoting to a dev-only console warning! :)

do you think this component would benefit from more comprehensive improvements (re: role="meter" vs role="progressbar") at a future time, or do you think it's at a good place now?

[kendallgassner]

do you think this component would benefit from more comprehensive improvements (re: role="meter" vs role="progressbar") at a future time, or do you think it's at a good place now?

Yes! I think it would be beneficial to investigate splitting up this component so that when progress is not dynamically changing we use role="meter"