fix: Standardize status icons and colors app-wide - #67
Conversation
vishu-bh
left a comment
vishu-bh
left a comment
There was a problem hiding this comment.
Solid PR!!
One thing to check src/components/mcp-servers/AdvancedSettings.tsx: still uses TriangleAlert. Should it use STATUS_ICON.warning?
0013056 to
6b2f5a8
Compare
gcgoncalves
left a comment
gcgoncalves
left a comment
There was a problem hiding this comment.
I pointed out three TriangleAlerts left behind. Not sure if I'm doing this right, if not, ignore. 😅
| }; | ||
| case "warning": | ||
| return { | ||
| Icon: TriangleAlert, |
| case "warning": | ||
| return { | ||
| labelId: "mcpServer.status.warning", | ||
| Icon: TriangleAlert, |
| }; | ||
| case "warning": | ||
| return { | ||
| Icon: TriangleAlert, |
7ed9667 to
3092dbf
Compare
Addressed! |
|
@a-effort - please review |
Signed-off-by: Marek Dano <mk.dano@gmail.com>
Signed-off-by: Marek Dano <mk.dano@gmail.com>
Signed-off-by: Marek Dano <mk.dano@gmail.com>
Signed-off-by: Marek Dano <mk.dano@gmail.com>
527fa47 to
bb72a3d
Compare
Signed-off-by: Marek Dano <mk.dano@gmail.com>
There was a problem hiding this comment.
Non-blocking notes, all fine as follow-ups:
1. Badge success/warning fail AA in light mode. bg-success/15 text-success puts green-600/amber-600 text on its own 15% tint: success 4.57:1 -> 2.79:1, warning 4.58:1 -> 2.71:1, on text-xs badge text needing 4.5:1. Dark mode is fine. One token can't serve both an icon fill (3:1) and text on its own tint (4.5:1); adding --success-foreground/--warning-foreground at the 700 shades for the badge, keeping text-success for icons, would cover both.
2. Two different reds in one form. MCPServerForm.tsx and QueryParameterAuth.tsx move * to text-destructive, but BasicAuth.tsx, OAuth2Auth.tsx (7x) and CustomHeadersAuth.tsx keep text-red-500. AdvancedSettings renders all four as siblings, so both reds show in the same form, and they diverge further in dark mode.
3. Three sibling preview panels missed. Same success/error header as PromptPreviewResult, still on CheckCircle2/AlertCircle: ResourcePreviewResult.tsx:97 (also text-emerald-500), ToolPreviewResult.tsx:61, ToolLiveInvokeResult.tsx:57. All present at the merge base.
4. Info notifications drop below AA. inline-notification.tsx applies STATUS_TONE_CLASS.info (text-muted-foreground) to the message as well as the icon: 5.17:1 -> 3.45:1 in light mode. Unaccented is right for the icon; the <p> could stay text-foreground.
4 participants
Closes #62
Summary
Standardizes the four status severities (success/info/warning/error) on the icon set already used in
ui/sonner.tsx(CircleCheckIcon/InfoIcon/TriangleAlertIcon/OctagonXIcon), and adds semantic--success/--warningCSS tokens alongside the existing--destructive, so color and glyph choices no longer drift per call site.src/lib/status.ts— a singleSTATUS_ICON/STATUS_TONE_CLASSlookup keyed by severity; most call sites now consume it instead of picking an icon/color ad hoc.--success/--warningtokens (light + dark) inindex.css, wired intobadge.tsxandStatusDot.tsx.StatusHeadline.tsxwhere warning and error rendered the identicalAlertTriangleicon.CheckCircle2,AlertTriangle,AlertCircle,XCircle,CircleAlert) onto the canonical set, and replaced raw Tailwind shades (green-500,emerald-500,yellow-300/500/600,red-500) with the semantic tokens across 17 components.CircleAlert); it's nowInfoIcon.Test plan
npm run lint,tsc -b,npm run format:checkall cleanvitest run— 184 files / 3062 tests passing (updated 2 tests that asserted old raw color classes; addedsrc/lib/status.test.tsfor the new lookup)