Skip to content

revert: restore the accordion trigger's hover underline #1145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vivek7405 merged 2 commits into from
Jul 27, 2026
Merged

revert: restore the accordion trigger's hover underline #1145

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4e4d93
Revert "fix(ui): make the accordion trigger hover a state layer (#1137)"
vivek7405 Jul 27, 2026
5412123
revert: restore the accordion trigger's hover underline
vivek7405 Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 2 additions & 26 deletions packages/ui/packages/registry/components/accordion.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* Chrome 120+, Safari 17.2+, Firefox 130+. Migrated from the prior
* <ui-accordion> custom element set.
*
* Design tokens used: --border, --ring, --accent, --accent-foreground.
* Design tokens used: --border, --ring, --foreground.
*
* @example
* ```html
Expand Down Expand Up @@ -67,31 +67,7 @@ export const accordionItemClass = (): string => 'group border-b last:border-b-0'
* prop combines both; native HTML has no `disabled` on <details>.
*/
export const accordionTriggerClass = (opts: { disabled?: boolean } = {}): string => {
// 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 <summary>, 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 -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 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';
const base = 'flex w-full cursor-pointer list-none items-center justify-between gap-4 py-4 text-left text-sm font-medium outline-none transition-all hover:underline 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;
};
Expand Down
Loading