Skip to content

fix: improve notifications styles - #95

Open
marekdano wants to merge 2 commits into
mainfrom
94-improve-notification-style
Open

fix: improve notifications styles#95
marekdano wants to merge 2 commits into
mainfrom
94-improve-notification-style

Conversation

@marekdano

Copy link
Copy Markdown
Contributor

Fixes four accessibility/consistency issues raised by @a-effort in review of #67 (tracked in #94).

  • Badge success/warning contrast (AA): bg-success/15 text-success put green-600/amber-600 text on its own 15% tint, dropping success to 2.79:1 and warning to 2.71:1 in light mode (text-xs badge text needs 4.5:1). Added --success-foreground/--warning-foreground tokens at the 700 shades for light mode (dark mode already passed, so it reuses the existing 400 shades) and pointed the badge's success/warning variants at them, while text-success/text-warning stay on the 600/400 shades for icon-only usages.
  • Two different reds in one form: BasicAuth.tsx, OAuth2Auth.tsx (8 occurrences), and CustomHeadersAuth.tsx used text-red-500 for required-field asterisks/errors while MCPServerForm.tsx and QueryParameterAuth.tsx used text-destructive. AdvancedSettings renders all of these as siblings, so both reds showed in the same form and diverged further in dark mode. All now use text-destructive.
  • Three sibling preview panels missed the status-icon pass: ResourcePreviewResult.tsx, ToolPreviewResult.tsx, and
    ToolLiveInvokeResult.tsx still used raw lucide CheckCircle2/AlertCircle (ResourcePreviewResult also used text-emerald-500) instead of the STATUS_ICON/text-success treatment PromptPreviewResult already had. Brought all three in line.
  • Info notifications dropped below AA: inline-notification.tsx applied STATUS_TONE_CLASS.info (text-muted-foreground) to both the icon and the message text, taking the message from 5.17:1 to 3.45:1 in light mode. The icon keeps the muted tone; the message now uses text-foreground.

Test plan

  • npx vitest run — 3278 passed, 1 pre-existing skip
  • npx eslint on changed files — clean
  • npx prettier --check on changed files — clean
  • npx tsc -b --noEmit — one pre-existing, unrelated error (ServerHandshakeRequest missing from generated types), confirmed present on
    main via git stash
  • Updated inline-notification.test.tsx's info-type assertion from text-muted-foreground to text-foreground to match the fix

Signed-off-by: Marek Dano <mk.dano@gmail.com>

@a-effort a-effort left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting one change, on the fourth item (info notification text):

The contrast figure behind it doesn't reproduce. --muted-foreground in light mode is oklch(0.556 0 0), which resolves to #737373; on --background that measures 4.74:1, not 3.45:1. The message is text-sm (14px, normal weight), so the bar is 4.5:1 and the existing styling already clears it. There is no AA problem to fix here.

Please drop from the PR:

  • src/components/ui/inline-notification.tsx:36, back to <p className={text-sm ${toneClass}}>
  • src/components/ui/inline-notification.test.tsx:57, assertion back to text-muted-foreground and the test name back to "neutral text"

My mistake; nice work! Test file I generated to take a look at changes, in case it's helpful for anyone:

color test.html

Signed-off-by: Marek Dano <mk.dano@gmail.com>
@marekdano
marekdano requested a review from a-effort September 3, 2026 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants