Skip to content

fix(ui): accordion trigger hover should be a state layer, not a dim #1136

Description

@vivek7405

Problem

The accordion trigger's hover treatment dims the label (text-foreground hover:text-foreground/80, landed in #1126 via d9b2fe1). That choice communicates with the wrong vocabulary, three ways:

  • It borrows the kit's disabled axis. opacity-50 is the disabled treatment in 18 places across the registry, including on this same trigger's disabled branch. Reduced prominence meaning both "unavailable" (50%) and "interactive, under your pointer" (80%), distinguished only by degree, is the ambiguity a design system exists to prevent.
  • It reduces contrast at the moment of interaction, which is directionally backwards for hover feedback.
  • It is off-pattern for disclosure headers. Material specifies a state layer (a background overlay of the on-surface colour), and Carbon, Polaris, and Atlassian all use a background tint on accordion headers. The kit itself agrees: hover:bg-accent is its most-used hover treatment for interactive rows (dropdown-menu items, tabs triggers).

Nothing has shipped: there has been no @webjsdev/ui release since d9b2fe1, so correcting it now is invisible to users; after the next release it becomes a visible change.

Design / approach

Replace the dim with the kit's own state-layer vocabulary:

  • Remove text-foreground and hover:text-foreground/80 from the base string.
  • Add rounded-md -mx-2 px-2 hover:bg-accent hover:text-accent-foreground.

Rationale for each piece: hover:bg-accent hover:text-accent-foreground is exactly the ghost-button / dropdown-item pair, so the trigger joins the existing vocabulary rather than adding a new one. -mx-2 px-2 keeps the label's left edge aligned with the panel content below (the content class has no horizontal padding either) while giving the tint breathing room around the text, the same pattern the webjs.dev docs sidebar links use. rounded-md keeps the tint from reading as a full-bleed table row between the item dividers, and improves the focus ring's shape as a side effect.

Removing the explicit text-foreground also reverts the small behaviour change #1126 introduced: the label goes back to inheriting its colour at rest, so a wrapper that sets a custom colour works again, and the hover pair overrides it only while hovered.

Implementation notes (for the implementing agent)

Where to edit:

  • packages/ui/packages/registry/components/accordion.ts, the base string inside accordionTriggerClass (~L77). Also rewrite the comment block above it (~L70-76): it currently narrates the dim rationale, which this change supersedes; keep the "no underline, this is a disclosure not a link" half, replace the dim half with the state-layer rationale.
  • After editing, run node website/scripts/copy-registry.mjs so the website's gitignored mirror (website/components/ui/accordion.ts) picks it up, then npm run css:build --workspace=@webjsdev/website so hover:bg-accent etc. are in the compiled stylesheet.

Landmines / gotchas:

  • The disabled branch appends pointer-events-none, which already suppresses hover; no change needed there, but confirm.
  • The gallery preview at /ui/accordion renders inside .ui-preview, which re-scopes --accent to the kit's neutral value (website/public/input.css). Verify the hover there AND in dark mode; the kit's dark --accent-shadcn is oklch(0.371).
  • webjs check was previously tripped by prose punctuation in comments in this file; keep AGENTS.md invariant 11 in mind when rewriting the comment.
  • The trigger is a <summary>; -mx-2 extends the tint 8px past the item dividers. That is the accepted look (same as the docs sidebar); do not "fix" it by adding horizontal padding without the negative margin, which would misalign the label from the panel content.

Invariants to respect:

  • shadcn API parity is about names and data attributes, not pixel-for-pixel presentation (packages/ui/AGENTS.md invariant 5); this is a presentation-only divergence like the underline removal before it.
  • packages/ stays plain .js/.ts-as-shipped; no new deps.

Tests + docs surfaces:

  • node --test packages/ui/test/registry-contents.test.js (hallmark-string smoke) must stay green.
  • Website suites (npm test --prefix website, npm run test:browser --workspace=@webjsdev/website) must stay green; the mirror is regenerated by pretest.
  • Verify visually at /ui/accordion in light and dark (hover a trigger; the row tints, text contrast does not drop).
  • No markdown doc describes the trigger's hover; nothing to sync beyond the in-file comment.

Acceptance criteria

  • Hovering an accordion trigger tints the row with bg-accent and does not reduce text contrast
  • The label inherits its colour at rest again (no explicit text-foreground)
  • Disabled triggers show no hover effect
  • Kit smoke tests and both website suites green
  • Verified in a real browser in light and dark, inside the gallery preview scope

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions