From fa52d0dadcbb6fa4a5c693c3737ddeca6f9c3f89 Mon Sep 17 00:00:00 2001 From: Vivek Date: Mon, 27 Jul 2026 22:59:40 +0530 Subject: [PATCH 1/2] fix(ui): make the accordion trigger hover a state layer Replaces the label dim that landed with #1126. Reduced prominence is this kit's disabled vocabulary (opacity-50, including on this very trigger), so dimming on hover said unavailable and under-your-pointer on the same axis, and it lowered text contrast at the exact moment of interaction. A background state layer is what Material, Carbon, and Polaris specify for disclosure headers, and it is already the kit's own most-used hover treatment (dropdown items, tabs), so the trigger now uses the same hover:bg-accent pair as a ghost button. The -mx-2/px-2 inset keeps the label aligned with the panel content while the tint breathes around it, rounded-md keeps the tint from reading as a table row, and dropping the explicit text-foreground restores rest-colour inheritance, undoing the small behaviour change the dim needed. Verified hovered in a real browser in light and dark inside the gallery preview scope. --- .../packages/registry/components/accordion.ts | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/packages/ui/packages/registry/components/accordion.ts b/packages/ui/packages/registry/components/accordion.ts index 04bc744f4..02fb11364 100644 --- a/packages/ui/packages/registry/components/accordion.ts +++ b/packages/ui/packages/registry/components/accordion.ts @@ -67,20 +67,26 @@ export const accordionItemClass = (): string => 'group border-b last:border-b-0' * prop combines both; native HTML has no `disabled` on
. */ export const accordionTriggerClass = (opts: { disabled?: boolean } = {}): string => { - // Hover dims the label instead of underlining it. An underline reads as a - // link affordance, and this is a disclosure control on a , not a - // navigation. A slight drop in text weight reads as "this responds to you" - // without borrowing another control's vocabulary, and it pairs with the - // chevron that rotates on open. (shadcn's React accordion underlines here; - // this is a deliberate divergence in presentation only, so the API parity - // that matters, variant and size names and the data attributes, is - // untouched.) + // Hover is a STATE LAYER (the ghost-button / dropdown-item pair), not an + // underline and not a text dim. An underline reads as a link affordance, + // and this is a disclosure control on a , not a navigation. A dim + // was tried and rejected: reduced prominence is this kit's DISABLED + // vocabulary (opacity-50, including on this very trigger), so dimming on + // hover said "unavailable" and "under your pointer" on the same axis, and + // it lowered text contrast at the exact moment of interaction. The tint + // also reveals the full-width hit area, which a text change cannot. + // (shadcn's React accordion underlines here; this is a deliberate + // divergence in presentation only, so the API parity that matters, variant + // and size names and the data attributes, is untouched.) // - // The label carries `text-foreground` explicitly rather than inheriting it, - // because the hover target has to be a known starting colour: inheriting an - // author's own colour and then dimming to 80% of the THEME foreground would - // shift the hue, not just the weight. - const base = 'flex w-full cursor-pointer list-none items-center justify-between gap-4 py-4 text-left text-sm font-medium text-foreground outline-none transition-all hover:text-foreground/80 focus-visible:ring-2 focus-visible:ring-ring/50 marker:hidden [&::-webkit-details-marker]:hidden'; + // The -mx-2/px-2 pair keeps the label's left edge aligned with the panel + // content below (which has no horizontal padding either) while giving the + // tint breathing room; the tint intentionally extends 8px past the item + // dividers, the same look as the webjs.dev docs sidebar links. rounded-md + // keeps it from reading as a full-bleed table row, and shapes the focus + // ring as a side effect. The label inherits its rest colour, so a wrapper + // that sets its own colour keeps it until hover. + const base = 'flex w-full cursor-pointer list-none items-center justify-between gap-4 rounded-md -mx-2 px-2 py-4 text-left text-sm font-medium outline-none transition-all hover:bg-accent hover:text-accent-foreground focus-visible:ring-2 focus-visible:ring-ring/50 marker:hidden [&::-webkit-details-marker]:hidden'; if (opts.disabled) return `${base} pointer-events-none cursor-not-allowed opacity-50`; return base; }; From bba0c4dfcb2f64a8ccbb8a74e557469e4dd5c877 Mon Sep 17 00:00:00 2001 From: Vivek Date: Mon, 27 Jul 2026 23:06:20 +0530 Subject: [PATCH 2/2] fix(ui): drop w-full from the trigger so the state layer is symmetric Review caught that width 100 percent plus symmetric negative margins is over-constrained, so the browser drops the end-side margin: the tint bled 8px past the left divider while stopping 8px short of the right one, and the chevron sat 16px in from where every other row ends. A summary styled as flex is a block-level box that already fills its containing block, so removing w-full lets both negative margins apply. Measured after: 8px overhang on both sides, label still aligned with the panel content. Also corrects the file's design-token doc line, which still declared the foreground token this change stopped using and omitted the two accent tokens it now does. --- .../packages/registry/components/accordion.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/ui/packages/registry/components/accordion.ts b/packages/ui/packages/registry/components/accordion.ts index 02fb11364..a134f9117 100644 --- a/packages/ui/packages/registry/components/accordion.ts +++ b/packages/ui/packages/registry/components/accordion.ts @@ -22,7 +22,7 @@ * Chrome 120+, Safari 17.2+, Firefox 130+. Migrated from the prior * custom element set. * - * Design tokens used: --border, --ring, --foreground. + * Design tokens used: --border, --ring, --accent, --accent-foreground. * * @example * ```html @@ -82,11 +82,16 @@ export const accordionTriggerClass = (opts: { disabled?: boolean } = {}): string // The -mx-2/px-2 pair keeps the label's left edge aligned with the panel // content below (which has no horizontal padding either) while giving the // tint breathing room; the tint intentionally extends 8px past the item - // dividers, the same look as the webjs.dev docs sidebar links. rounded-md - // keeps it from reading as a full-bleed table row, and shapes the focus - // ring as a side effect. The label inherits its rest colour, so a wrapper - // that sets its own colour keeps it until hover. - const base = 'flex w-full cursor-pointer list-none items-center justify-between gap-4 rounded-md -mx-2 px-2 py-4 text-left text-sm font-medium outline-none transition-all hover:bg-accent hover:text-accent-foreground focus-visible:ring-2 focus-visible:ring-ring/50 marker:hidden [&::-webkit-details-marker]:hidden'; + // dividers on BOTH sides, the same look as the webjs.dev docs sidebar + // links. There is deliberately no w-full here: a summary styled as flex is + // a block-level box that already fills its containing block, and width:100% + // plus symmetric negative margins is over-constrained, so the browser drops + // the end-side margin and the tint bleeds left while stopping short on the + // right (mirrored in RTL). rounded-md keeps the tint from reading as a + // full-bleed table row, and shapes the focus ring as a side effect. The + // label inherits its rest colour, so a wrapper that sets its own colour + // keeps it until hover. + const base = 'flex cursor-pointer list-none items-center justify-between gap-4 rounded-md -mx-2 px-2 py-4 text-left text-sm font-medium outline-none transition-all hover:bg-accent hover:text-accent-foreground focus-visible:ring-2 focus-visible:ring-ring/50 marker:hidden [&::-webkit-details-marker]:hidden'; if (opts.disabled) return `${base} pointer-events-none cursor-not-allowed opacity-50`; return base; };