Skip to content

Conversation

dchyun
Copy link
Contributor

@dchyun dchyun commented Jan 13, 2025

📌 Summary

This PR implements usage of the aria-controls attribute in the DisclosurePrimitive component to align show/hide behavior across the repo as follows in the initiative from HDS-3581.

The DisclosurePrimitive is leveraged in the Accordion and Reveal.

🛠️ Detailed description

Currently in the DisclosurePrimitive, the toggled content does not follow the proper a11y structure for toggled content, or leverage aria-controls in the preferred way.

As per a11y guidance in HDS-3581, all togged content should follow a pattern leveraging aria-controls, and all toggled containers should not be removed from the DOM on each toggle. They should always be present, and only the content inside them is added and removed.

Currently the DisclosurePrimitive follows a pattern where the entire content block is added and removed on each toggle, and any aria-controls attributes are set to the id of this removable block. In this PR the hds-disclosure-primitive__content block is now in the DOM at all times, and the yielded content is removed or added based on the toggle.

In this PR, in the Accordion and Reveal, the aria-controls attribute of the toggle elements are now set equal to a contentId provided by the DisclosurePrimitive. They are no longer set to the id of content inside the yielded block.

Smoke Testing

Since this PR involves updating the DOM structure of the DisclosurePrimitive it has been smoke tested in various products to ensure they are not relying on the existing structure in any of their tests.

  • Cloud UI
  • Atlas
    • Note: The one failing test is unrelated to these changes
  • ✅ Nomad
    • There was one test case relying on the old structure, but that test has been updated in a PR

📸 Screenshots

Before - Accordion

Closed
Screenshot 2025-01-10 at 3 08 59 PM

Open
Screenshot 2025-01-10 at 3 09 14 PM

After - Accordion

Closed
Screenshot 2025-01-10 at 3 16 11 PM

Open
Screenshot 2025-01-10 at 3 16 23 PM

🔗 External links

Jira ticket: HDS-4324


👀 Component checklist

💬 Please consider using conventional comments when reviewing this PR.

Copy link

vercel bot commented Jan 13, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
hds-showcase ✅ Ready (Inspect) Visit Preview May 27, 2025 6:23pm
hds-website ✅ Ready (Inspect) Visit Preview May 27, 2025 6:23pm

@dchyun dchyun marked this pull request as ready for review January 13, 2025 15:21
@dchyun dchyun requested a review from a team as a code owner January 13, 2025 15:21
@dchyun dchyun changed the title Disclosure Primitive - Implement a11y toggled content pattern DisclosurePrimitive - Implement a11y toggled content pattern Jan 13, 2025
shleewhite
shleewhite previously approved these changes Jan 13, 2025
Copy link
Contributor

@shleewhite shleewhite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, but we should definitely test this in products before we ship. Last year we changed something similar-ish in Dropdown and it broke tests (and led us to add @preserveContentInDom):

{{#if (or PP.isOpen @preserveContentInDom)}}
)

@dchyun
Copy link
Contributor Author

dchyun commented Jan 13, 2025

This looks good, but we should definitely test this in products before we ship. Last year we changed something similar-ish in Dropdown and it broke tests (and led us to add @preserveContentInDom):

{{#if (or PP.isOpen @preserveContentInDom)}}

)

Tested the changes across atlas and cloud-ui and didn't run into any test failures. But I think this is an open question if these kind of DOM changes could be considered breaking or not. @alex-ju do you see any potential issues with this or think it could possibly be a breaking change based on your recent RFC?

Similar DOM changes will have to be made for the hds-tooltip updates, so can follow whatever we decide here on that as well.

@dchyun dchyun marked this pull request as draft January 16, 2025 18:30
@dchyun
Copy link
Contributor Author

dchyun commented Jan 16, 2025

Converted back to a draft pr until issues with failing tests in consumer's repos can be sorted.

Copy link
Contributor

📦 RC Packages Published

Latest commit: f30552b

Published 1 packages

@hashicorp/[email protected]

yarn up -C @hashicorp/design-system-components@rc

@didoo didoo removed this from the [email protected] milestone Jun 2, 2025
@dchyun dchyun merged commit 895095d into main Jun 5, 2025
23 of 25 checks passed
@dchyun dchyun deleted the dchyun/disclosure-primitive-aria-controls branch June 5, 2025 15:53
@hashibot-hds hashibot-hds mentioned this pull request Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants