Skip to content

Conversation

annawanggg
Copy link
Contributor

@annawanggg annawanggg commented Aug 5, 2025

📌 Summary

If merged, this PR adds a11y guardrails to the Dropdown ToggleIcon component, so the consumer can only set @hasChevron to false if the icon is in a list of allowed values.

🛠️ Detailed description

  • Adds a check/assertion to @hasChevron
  • Adds allowed icon list as a type (so the list can be expanded later if needed, and we only need to update the list)
  • Adds a test to check that an assertion is thrown if the component is used incorrectly

📸 Screenshots

Tests pass.
Screenshot 2025-08-05 at 7 24 00 PM
Screenshot 2025-08-05 at 7 24 15 PM

🔗 External links

Jira ticket: HDS-5265


👀 Component checklist

💬 Please consider using conventional comments when reviewing this PR.

📋 PCI review checklist
  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
  • If applicable, I've worked with GRC to document the impact of any changes to security controls.
    Examples of changes to controls include access controls, encryption, logging, etc.
  • If applicable, I've worked with GRC to ensure compliance due to a significant change to the in-scope PCI environment.
    Examples include changes to operating systems, ports, protocols, services, cryptography-related components, PII processing code, etc.

Copy link

vercel bot commented Aug 5, 2025

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

Name Status Preview Updated (UTC)
hds-showcase ✅ Ready (Inspect) Visit Preview Aug 6, 2025 4:24pm
hds-website ✅ Ready (Inspect) Visit Preview Aug 6, 2025 4:24pm

Copy link

hashicorp-cla-app bot commented Aug 5, 2025

CLA assistant check
All committers have signed the CLA.

Co-authored-by: Anna Wang <[email protected]>

Failing test fixed and reworked the hasChevron() function to more closely align with the syntax of other functions
@hashibot-hds hashibot-hds added the docs-website Content updates to the documentation website label Aug 6, 2025
@annawanggg annawanggg changed the title HDS-5265: a11y guardrails for dropdown toggle icon HDS-5265: Adds a11y guardrails in Dropdown ToggleIcon component Aug 6, 2025
@annawanggg annawanggg changed the title HDS-5265: Adds a11y guardrails in Dropdown ToggleIcon component HDS-5265: Adds a11y guardrails in Dropdown ToggleIcon component Aug 6, 2025
Acceptable value: any [icon](/icons/library) name.
</C.Property>
<C.Property @name="hasChevron" @type="boolean" @default="true">
Per design, `false` is only currently allowed when the "more-horizontal" or "more-vertical" icons are used; it is set to `true` by default.
Copy link
Contributor Author

@annawanggg annawanggg Aug 6, 2025

Choose a reason for hiding this comment

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

In this PR, we include both more-horizontal and more-vertical in the allowed icon list, but existing documentation seems to only refer to more-horizontal being permitted without chevrons (when used as an overflow menu inTable components).

Just to check, it's okay to have more-vertical in the allowed list, and update documentation to reflect this?

Copy link
Contributor

Choose a reason for hiding this comment

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

We didn't have both when we first made this component, I think, but @hashicorp/hds-design could confirm.

Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned during our HDS Engineering sync today, better to involve the HDS designers on this, to discuss/decide which icons are "allowed" to have when the chevron is not visible. We can't make an unilateral decision on this (even if it's documented, better to double check with them).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes agreed! Just marked the PR for review so everyone can discuss :)

@shleewhite shleewhite added this to the [email protected] milestone Aug 6, 2025
@annawanggg annawanggg marked this pull request as ready for review August 6, 2025 16:01
@annawanggg annawanggg requested review from a team as code owners August 6, 2025 16:01
Copy link
Contributor

@jorytindall jorytindall left a comment

Choose a reason for hiding this comment

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

Thanks for looping @hashicorp/hds-design in! I think this change is logical given how we expect this component to be used, and I can't think of any other icons that would fall into this category at this time.

However, where this gives me pause is whether this is a guidance over control question. The reasoning behind supporting this variant when used in the table is because this is a common pattern across the web for consolidating options and CRUD actions in dense UIs, and while I can't think of other icon examples right now, I imagine there are others that are logical given the context.

I'll invite other designers to chime in with opinions, we discussed this briefly today, but were missing a few folks that would benefit the discussion. I might request holding off until after our design sync later this week for us to discuss it with a full quorum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-website Content updates to the documentation website packages/components showcase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants