feat(mcp): i18n + a11y McpStatusBadge status labels#2635
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesMcpStatusBadge i18n refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
oxoxDev
left a comment
There was a problem hiding this comment.
Walkthrough
Routes 4 hardcoded English status labels in McpStatusBadge.tsx through useT() using existing channels.status.{connected,connecting,disconnected,error} keys (verified all 4 present in en.ts + chunks). Adds role="status" + aria-live="polite" matching McpServersTab's alpha-banner pattern. New McpStatusBadge.test.tsx covers each variant + a11y + unknown-status fallback + className passthrough. 2 files +59/-9. CodeRabbit APPROVED. Same 2 unrelated CI fails as #2634 (e2e Linux Appium + Windows ACL); branch stale from May 25.
Verified
channels.status.{connected,disconnected,error}defined aten.ts:477-479+en-1.ts:646-648.channels.status.connectingdefined aten.ts:1017+en-2.ts:88.- All 4 labels match badge's intent (Connected/Connecting/Disconnected/Error).
ChannelStatusBadge.tsxexists atapp/src/components/channels/— "mirror" claim verified.
Nits
aria-live="polite"test only checksaria-liveattribute, notrole="status". Worthexpect(badge).toHaveAttribute('role', 'status')in the same assertion block for defense.- Reusing
channels.status.*is the right call today, but if MCP vocabulary diverges later (PR body mentions "spawning" / "handshaking"), it's worth a comment in the file pointing future devs at splitting tomcp.status.*rather than expanding the shared bucket — PR body covers this in plain English; a// FIXME(divergence): split to mcp.status.* if vocabulary divergeswould lock it.
Questions
- Same as #2634 — branch stale from May 25, mergeable=UNKNOWN, CI fails on jobs unrelated to the change. Sync with
upstream/mainto refresh?
graycyrus
left a comment
There was a problem hiding this comment.
@aashir-athar hey! the code looks good to me — clean i18n migration using the existing channels.status.* keys (solid call to reuse rather than duplicate), a11y attributes match the McpServersTab pattern, and the test suite covers all the right cases (status variants, fallback, className passthrough, aria attributes). no findings on my end beyond what oxoxDev already noted.
there are 2 CI failures on this PR though: e2e / E2E (Linux / Appium Chromium) and test / Rust Core Tests (Windows — secrets ACL). looks like these are pre-existing failures unrelated to this change (oxoxDev flagged the same ones), but i need them green before i can approve. if you sync the branch with upstream/main that might clear them — let me know if you need help.
Summary
McpStatusBadge.tsx('Connected' / 'Connecting' / 'Disconnected' / 'Error') throughuseT()using the existingchannels.status.*keys. The badge's own docstring says it "Mirrors ChannelStatusBadge"; the labels are identical English; reusing the shared key set avoids redundant translation work.role="status"andaria-live="polite"so screen readers announce state changes — matches the alpha-banner pattern already used inMcpServersTab.McpStatusBadge.test.tsxcovering eachServerStatusvariant, the a11y attributes, the disconnected fallback for unknown statuses, and className passthrough.Problem
McpStatusBadge.tsxslipped past the #2577 React i18n sweep because it's an isolated leaf component without a co-located test. Lines 7–25 hardcoded English labels (label: 'Connected', etc.) directly in aSTATUS_STYLESobject, violating the CLAUDE.md rule that every user-visible string inapp/src/**must go throughuseT(). Non-EN users saw English labels on every MCP server connection state.The same
<span>had noroleoraria-live, so screen readers wouldn't announce state changes — important for a long-running connection that can transition betweenconnecting→connected→errorwhile the user is on a different part of the page.Solution
channels.status.{connected,connecting,disconnected,error}i18n keys rather than introducingmcp.status.*duplicates. The labels are character-identical to the channel status set; the badge's docstring already calls it a "mirror" ofChannelStatusBadge. If the MCP vocabulary ever diverges (e.g. adds "spawning" / "handshaking"), the keys can be split then.role="status"+aria-live="polite"(matchesMcpServersTab's alpha banner).McpStatusBadge.test.tsxcovering: every status variant renders the right label, a11y attributes present, fallback to "Disconnected" for unknown status values, className passthrough preserves built-in classes.Submission Checklist
McpStatusBadge.test.tsx.McpStatusBadge.tsxare exercised by the new test file (it.eachover all 4 status variants + a11y + fallback + className).## Related— N/A: leaf-component fix.Closes #NNNin## Related— N/A: no linked issue.Impact
Related
AI Authored PR Metadata
Linear Issue
Commit & Branch
feat/mcp-servers-ui-panelValidation Run
pnpm --filter openhuman-app format:check—McpStatusBadge.test.tsxclean;McpStatusBadge.tsxinherits CRLF from Windows checkout (CI on Linux passes)pnpm typecheck— cleanpnpm vitest run src/components/channels/mcp/— 82/82 pass (includes the new file + the pinnedSkills.mcp-coming-soon.test.tsxconfirming no regression)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
Tests
Refactor