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

Smoke testing

  • Cloud UI: 🟒
  • Atlas: 🟑
    • there is 1 instance of a failing dropdown and it is the user menu in the mobile view header (screenshot is what it looks like with the chevron). it should be safe to add the chevron bc when the dropdown is in desktop view there is one.
Screenshot 2025-10-16 at 2 10 18β€―PM
  • Boundary: 🟒
  • Vault: I manually checked and there are no instances of Dropdowns with @hasChevron={{false}} with an invalid icon

πŸ”— 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.

@vercel
Copy link

vercel bot commented Aug 5, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
hds-showcase Ready Ready Preview Oct 16, 2025 7:13pm
hds-website Ready Ready Preview Oct 16, 2025 7:13pm

@hashicorp-cla-app
Copy link

hashicorp-cla-app bot commented Aug 5, 2025

CLA assistant check
All committers have signed the CLA.

@hashicorp hashicorp deleted a comment from hashicorp-cla-app bot Aug 6, 2025
@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 :)

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.

@shleewhite shleewhite added the release-candidate Publishes release candidates to npm label Oct 14, 2025
@github-actions
Copy link
Contributor

πŸ“¦ RC Packages Published

Latest commit: fb70667

Published 1 packages

@hashicorp/[email protected]

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

feat: update enum and type names for allowed icons

tests pass (#3089)

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

feat: update docs to mention more-vertical as well

feat: added changeset

fix: edited changeset formatting to be more consistent

fix: type of allowed icons array

fix: website indentation issue
@shleewhite shleewhite force-pushed the annawanggg/dropdown-a11y branch from a8d64c1 to 69ee082 Compare October 16, 2025 19:06
Copy link
Contributor

@MelSumner MelSumner left a comment

Choose a reason for hiding this comment

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

πŸŽ‰ πŸ‘

@shleewhite shleewhite requested a review from didoo October 17, 2025 19:40
@shleewhite shleewhite merged commit 14945bd into main-5.0.0 Oct 23, 2025
14 of 16 checks passed
@shleewhite shleewhite deleted the annawanggg/dropdown-a11y branch October 23, 2025 22:58
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.

7 participants