fix: improve notifications styles - #95
Open
marekdano wants to merge 1 commit into
Open
Conversation
Signed-off-by: Marek Dano <mk.dano@gmail.com>
a-effort
requested changes
Sep 3, 2026
Contributor
There was a problem hiding this comment.
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 totext-muted-foregroundand 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:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes four accessibility/consistency issues raised by @a-effort in review of #67 (tracked in #94).
bg-success/15 text-successput 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-xsbadge text needs 4.5:1). Added--success-foreground/--warning-foregroundtokens at the 700 shades for light mode (dark mode already passed, so it reuses the existing 400 shades) and pointed the badge'ssuccess/warningvariants at them, whiletext-success/text-warningstay on the 600/400 shades for icon-only usages.BasicAuth.tsx,OAuth2Auth.tsx(8 occurrences), andCustomHeadersAuth.tsxusedtext-red-500for required-field asterisks/errors whileMCPServerForm.tsxandQueryParameterAuth.tsxusedtext-destructive.AdvancedSettingsrenders all of these as siblings, so both reds showed in the same form and diverged further in dark mode. All now usetext-destructive.ResourcePreviewResult.tsx,ToolPreviewResult.tsx, andToolLiveInvokeResult.tsxstill used raw lucideCheckCircle2/AlertCircle(ResourcePreviewResultalso usedtext-emerald-500) instead of theSTATUS_ICON/text-successtreatmentPromptPreviewResultalready had. Brought all three in line.inline-notification.tsxappliedSTATUS_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 usestext-foreground.Test plan
npx vitest run— 3278 passed, 1 pre-existing skipnpx eslinton changed files — cleannpx prettier --checkon changed files — cleannpx tsc -b --noEmit— one pre-existing, unrelated error (ServerHandshakeRequestmissing from generated types), confirmed present onmainviagit stashinline-notification.test.tsx's info-type assertion fromtext-muted-foregroundtotext-foregroundto match the fix