feat(desktop): TopBar connection status badge#36
Conversation
Add a 10px status dot to TopBar (§8.3 of docs/research/18-ux-master-plan.md): - Green / amber / red / grey states based on last connection test result - Hover tooltip shows state label + last-tested time + error message + re-test prompt - Click triggers an active-provider connection test via new IPC connection:v1:test-active - Auto-test on mount if status is stale (>5 min) or untested - connectionStatus slice added to Zustand store with setConnectionStatus + testConnection actions - New IPC handler reads active provider credentials from encrypted keychain (no key from renderer) - i18n keys added under topbar.status.* in en.json and zh-CN.json - Unit tests for store actions and component color/time helpers (98 tests total, all green) Signed-off-by: hqhq1025 <1506751656@qq.com>
There was a problem hiding this comment.
Findings
-
[Major] Renderer bridge failures are mislabeled as
no_provider(silent misdiagnosis) — this hides IPC/preload breakage as a config issue, making real runtime failures harder to detect and violating the “no silent fallbacks” constraint. Evidenceapps/desktop/src/renderer/src/store.ts:272
Suggested fix:async testConnection() { const cfg = get().config; if (!window.codesign) { const msg = tr('errors.rendererDisconnected'); set({ connectionStatus: { state: 'error', lastTestedAt: Date.now(), lastError: msg }, errorMessage: msg, lastError: msg, }); return; } if (cfg === null || !cfg.hasKey || cfg.provider === null) { set({ connectionStatus: { state: 'no_provider', lastTestedAt: null, lastError: null } }); return; } // ... }
-
[Major] Relative time text is hardcoded in English and bypasses i18n — Chinese locale will still show
"30s ago"/"3m ago", creating mixed-language UI. Evidenceapps/desktop/src/renderer/src/components/ConnectionStatusDot.tsx:7
Suggested fix:function formatRelativeTime(ts: number): string { const diffSec = Math.round((Date.now() - ts) / 1000); if (diffSec < 60) return t('topbar.status.relative.secondsAgo', { count: diffSec }); const diffMin = Math.round(diffSec / 60); if (diffMin < 60) return t('topbar.status.relative.minutesAgo', { count: diffMin }); return t('topbar.status.relative.hoursAgo', { count: Math.round(diffMin / 60) }); }
-
[Major] New UI uses raw pixel typography value
text-[11px], which violates the token-only UI value constraint — this introduces a non-token size in app code. Evidenceapps/desktop/src/renderer/src/components/ConnectionStatusDot.tsx:86
Suggested fix:<span role="tooltip" className="... text-[var(--text-xs)] ..." >
-
[Minor] “Component tests” do not exercise production component logic — test file redefines helper logic locally, so regressions in
ConnectionStatusDot.tsxcan pass undetected. Evidenceapps/desktop/src/renderer/src/components/ConnectionStatusDot.test.ts:4
Suggested fix:import { render, screen } from '@testing-library/react'; import { ConnectionStatusDot } from './ConnectionStatusDot'; it('renders localized last-tested tooltip', () => { render(<ConnectionStatusDot />); expect(screen.getByRole('button')).toHaveAttribute('aria-label', expect.stringContaining('Last tested')); });
Summary
- Review mode: initial
- 4 issues found in changed lines (3 Major, 1 Minor). No new dependency/license or direct provider SDK import issues found in this diff.
docs/VISION.mdanddocs/PRINCIPLES.mdare not present in this checkout (as noted byCLAUDE.md), so those docs could not be re-validated here.
Testing
- Not run (automation)
open-codesign Bot
|
|
||
| async testConnection() { | ||
| const cfg = get().config; | ||
| if (!window.codesign || cfg === null || !cfg.hasKey || cfg.provider === null) { |
There was a problem hiding this comment.
[Major] !window.codesign is mapped to no_provider, which masks renderer/main bridge failures as config state. Please surface this as an explicit error state (e.g., rendererDisconnected) instead of provider-missing.
|
|
||
| const STALE_MS = 5 * 60 * 1000; | ||
|
|
||
| function formatRelativeTime(ts: number): string { |
There was a problem hiding this comment.
[Major] Relative-time text is hardcoded in English (s/m/h ago) and bypasses i18n, so zh-CN users will see mixed-language tooltip content. Please localize these units/phrases via translation keys.
| </button> | ||
| <span | ||
| role="tooltip" | ||
| className="pointer-events-none absolute bottom-full mb-1.5 left-1/2 -translate-x-1/2 z-50 whitespace-nowrap max-w-xs rounded-[var(--radius-sm)] bg-[var(--color-text-primary)] px-2 py-1 text-[11px] font-medium text-[var(--color-background)] opacity-0 transition-opacity duration-150 delay-[400ms] group-hover:opacity-100 shadow-[var(--shadow-card)]" |
There was a problem hiding this comment.
[Major] text-[11px] introduces a raw pixel typography value in app code. Per token-only UI constraints, use a tokenized text size (e.g., var(--text-*)) from packages/ui tokens.
| import { describe, expect, it } from 'vitest'; | ||
| import type { ConnectionState } from '../store'; | ||
|
|
||
| // ── Pure helpers extracted for unit testing ──────────────────────────────── |
There was a problem hiding this comment.
[Minor] This test file redefines helper logic locally instead of importing/testing the production component logic, so regressions in ConnectionStatusDot.tsx can slip through. Please switch to component-level assertions or exported helpers from the source file.
Summary
Implements §8.3 of
docs/research/18-ux-master-plan.md.connection:v1:test-activeIPC — reads active provider credentials from the encrypted keychain in the main process (no key passed from renderer)connectionStatus: { state, lastTestedAt, lastError }+setConnectionStatus+testConnectionactionstopbar.status.*inen.jsonandzh-CN.jsonTest plan
pnpm -r typecheck)pnpm lint)pnpm test)