Add details-toggle Catalyst element to improve accessibility of Details component

[ktravers]

Authors: Please fill out this form carefully and completely.

Reviewers: By approving this Pull Request you are approving the code change, as well as its deployment and mitigation plans.
Please read this description carefully. If you feel there is anything unclear or missing, please ask for updates.

What are you trying to accomplish?

This PR adds a new Catalyst element, details-toggle, to the Details component in order to ensure that when a Details component is toggled open/closed, its aria attributes are updated as expected by default.

Previously, aria-expanded and aria-label attributes were not included at all, which made it difficult for screen reader users interact with a rendered Details component, given there was no indication what would happen when the component was activated.

Note

For GitHub staff reviewers: these changes have been upstreamed from dotcom. See https://github.com/github/github/pull/357658.

Screenshots

Screen.recording.Details.component.aria.attribute.changes.mov

Integration

Yes, but only for github/github. When this version of @primer/view-components is released, we can deprecate the Catalyst element added in https://github.com/github/github/pull/357658. Note that this change isn't technically required, in that nothing will break if we delay or even skip removing the gh/gh component. It's purely code cleanup, removing duped/unnecessary code to improve general maintainability.

List the issues that this change affects.

Related to https://github.com/github/accessibility-audits/issues/10012

Risk Assessment

• Low risk the change is small, highly observable, and easily rolled back.
• Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
• High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

What approach did you choose and why?

I followed existing conventions for this codebase and created a Catalyst component for updating the aria attributes when the details component is toggled open/closed.

Re: visual changes -- These are intentional. They're present because I added reasonable default aria label values to the Primer::Alpha::Dropdown component, which renders Primer::Beta::Details. So Dropdown will also benefit from these accessibility improvements.

Anything you want to highlight for special attention from reviewers?

Nothing in particular.

Accessibility

• Fixes axe scan violation - This change fixes an existing axe scan violation.
• No new axe scan violation - This change does not introduce any new axe scan violations.

Merge checklist

• Added/updated tests
• Added/updated documentation
• Added/updated previews (Lookbook)
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

[changeset-bot]

🦋 Changeset detected

Latest commit: 073898f

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

This PR includes changesets to release 1 package

Name	Type
@primer/view-components	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]

⚠️ Visual or ARIA snapshot differences found

Our visual and ARIA snapshot tests found UI differences. Please review the differences by viewing the files changed tab to ensure that the changes were intentional.

Review differences

[ktravers]

Taking this back to draft while I investigate visual differences.

[camertron]

Thanks for this! 🎉 What would you think about adding some system tests for this that check the aria labels when the elements are open and closed?

[ktravers]

What would you think about adding some system tests for this that check the aria labels when the elements are open and closed?

@camertron No problem! I was hoping the Playwright snapshot tests would be enough, but you're right, that's not the same thing as proper system tests.

I'll take this back to draft while adding those 👍

[ktravers]

@camertron System tests added in 073898f

[camertron]

Sweet, thank you!