Skip to content

fix(ui): destructive menu item is 4.47:1 on its hover tint under theme-stone #1143

Description

@vivek7405

Problem

The destructive dropdown-menu item's own red text sits ON its hover tint, so the two are only as far apart as the tint is heavy. Under theme-stone, whose dark popover is lighter than neutral's, shadcn's dark:bg-destructive/20 puts them at 4.47:1, below the 4.5:1 minimum.

packages/ui/packages/registry/components/dropdown-menu.ts:95
  data-[variant=destructive]:text-destructive
  dark:data-[variant=destructive]:focus:bg-destructive/20
  dark:data-[variant=destructive]:hover:bg-destructive/20

This is inherited from shadcn, not introduced by #1138 / #1139. Measured against the pre-#1138 token (oklch(0.704 0.191 22.216)), the same pairing was already 4.49:1. The token work changed it by two hundredths.

Margins across the seven base colours at the shipped token, dark mode:

base ratio
stone 4.47
zinc 4.58
neutral, mauve, olive, mist, taupe 4.62

Light mode is fine at /10 (5.16:1 everywhere).

Design / approach

Lowering the dark tint from /20 to /15 clears every base at 4.89:1 worst case, and the tint stays a more visible hover than the light-mode /10 beside it (OKLab ΔE 0.086 against the bare popover, versus 0.059 for the light tint and 0.030 for the kit's ordinary hover:bg-accent). So the affordance is not lost.

The reason this is its own issue rather than part of #1139: it means diverging from shadcn on a class string, and packages/ui/AGENTS.md invariant 5 treats a divergence as a decision to record deliberately rather than fold into an unrelated colour change. It was implemented and then reverted in #1139 for exactly that reason (see commits 84357be and a6d88e1).

Alternatives, if the divergence is unwanted:

  • Lighten --destructive in dark. Rejected in fix(ui): make the destructive red legible in dark mode #1139: it changes a colour every destructive surface uses to fix one component, and it visibly washes out the destructive button.
  • Give theme-stone a --destructive override. Keeps parity on the class string but adds a per-theme special case, and the other six bases sit at 4.62, close enough that a future popover tweak reopens this.
  • Accept 4.47:1. Defensible only if the item is treated as a UI component rather than text, which it is not: it is a text label.

Implementation notes (for the implementing agent)

Where to edit:

  • packages/ui/packages/registry/components/dropdown-menu.ts:95, the two dark:data-[variant=destructive]:*:bg-destructive/20 classes in dropdownMenuItemClass. Both must move together.
  • packages/ui/AGENTS.md invariant 5, the deliberate-divergences list, if the /15 route is taken.
  • packages/ui/test/destructive-contrast.test.js, the test currently marked todo: 'tracked separately' at the bottom. Drop the todo flag once it passes.

Landmines / gotchas:

  • The test derives the alpha from the class string via menuTintAlpha() rather than hardcoding it, so changing the component is enough. It also asserts the focus and hover variants agree, so changing only one fails loudly.
  • Verify by computation, not by eye. 4.47:1 and 4.89:1 are visually indistinguishable, which is why this sat unnoticed.
  • --popover equals --card in the neutral theme, so a neutral-only check hides the stone case entirely. Run all seven bases.

Invariants to respect:

  • packages/ui/AGENTS.md invariant 5. Variant names, size names, and data attributes must not change; only presentation may.
  • Repo root AGENTS.md invariant 11 for any prose added.

Tests + docs surfaces:

  • packages/ui/test/destructive-contrast.test.js already covers this pairing across all seven bases in both modes.
  • No docs-site or gallery surface documents this tint.

Acceptance criteria

  • The destructive menu item clears 4.5:1 on its hover tint across all seven base colours in both modes
  • The hover tint remains visibly distinguishable from the unhovered popover
  • The todo flag is removed from the test, so the pairing is asserted rather than reported
  • If the divergence route is taken, it is recorded in packages/ui/AGENTS.md invariant 5 with the reason at the call site

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