feat(tray): support multi-model usage display and tray controls#232
feat(tray): support multi-model usage display and tray controls#232Zzzia wants to merge 4 commits intorobinebers:mainfrom
Conversation
Zzzia
requested review from
davidarny,
robinebers and
validatedev
as code owners
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 20 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/pages/settings.tsx">
<violation number="1" location="src/pages/settings.tsx:332">
P2: The new tray-lines content is outside the element registered with `setNodeRef`, so the draggable/sortable item only covers the header row. DnD-kit measures and transforms only the `setNodeRef` element, which can make the header drag independently and cause incorrect spacing when tray lines are visible.</violation>
</file>
<file name="src/lib/settings.ts">
<violation number="1" location="src/lib/settings.ts:145">
P2: normalizePluginSettings spreads optional settings.trayLines without a default. When callers pass settings without trayLines (as in existing tests), this throws a TypeError at runtime.</violation>
<violation number="2" location="src/lib/settings.ts:174">
P2: `arePluginSettingsEqual` treats different `trayLines` key sets as equal when arrays are empty because it never verifies that `b` contains the same keys as `a`. This can skip saving normalized settings when tray line keys change.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Zzzia
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds multi-model system tray usage rendering and settings to control which progress lines appear per provider, plus a global “show tray icon” toggle. It extends plugin metadata usage (primary candidates) and updates tray icon rendering to support a compact grid display with tooltips.
Changes:
- Add persisted settings for tray icon visibility (
showTrayIcon) and per-plugin selected tray lines (pluginSettings.trayLines). - Update tray primary progress selection to return multiple labeled items, and update tray icon SVG rendering to support up to 4 grid cells (with optional hidden provider icon).
- Expose UI controls in Settings for “Show tray icon” and per-provider “Tray lines” checkboxes; update associated hooks/tests.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/stores/app-preferences-store.ts | Adds showTrayIcon to app preferences store state/actions. |
| src/pages/settings.tsx | Adds System Tray section and per-plugin tray line selection UI; updates plugin list item layout. |
| src/pages/settings.test.tsx | Updates settings page tests for new plugin config fields (but missing new required tray icon props). |
| src/lib/tray-primary-progress.ts | Changes tray primary progress from single fraction to multiple labeled items; reads trayLines. |
| src/lib/tray-primary-progress.test.ts | Updates expectations for the new { items: [...] } return shape. |
| src/lib/tray-bars-icon.ts | Reworks tray icon SVG layout to render a compact text grid and optionally hide the provider icon. |
| src/lib/tray-bars-icon.test.ts | Adds coverage for new grid rendering behavior and hidden-icon non-zero sizing. |
| src/lib/settings.ts | Adds persistence for trayLines in plugin settings and showTrayIcon as a global preference. |
| src/lib/settings.test.ts | Updates tests for trayLines and new plugin meta shape. |
| src/hooks/app/use-tray-icon.ts | Updates tray update pipeline to render multi-cell icons, clear native title, and set tooltips. |
| src/hooks/app/use-settings-system-actions.ts | Adds handler to persist showTrayIcon and trigger tray refresh. |
| src/hooks/app/use-settings-plugin-list.ts | Surfaces primaryCandidates and current trayLines to settings UI. |
| src/hooks/app/use-settings-plugin-list.test.ts | Updates expectations for new fields in settings plugin list state. |
| src/hooks/app/use-settings-plugin-actions.ts | Adds action to toggle per-plugin tray line selections and persist them. |
| src/hooks/app/use-settings-bootstrap.ts | Loads persisted showTrayIcon on startup and applies to store. |
| src/components/app/app-content.tsx | Threads new settings actions/values through to SettingsPage. |
| src/App.tsx | Wires showTrayIcon into tray hook and settings actions into app props. |
| src/App.test.tsx | Updates tray-related expectations (gridCells, title clearing behavior) and plugin settings shape. |
| plugins/codex/plugin.json | Adds/adjusts primaryOrder on multiple progress lines to support multi-line tray candidates. |
| plugins/antigravity/plugin.json | Adds primaryOrder to additional progress lines to support multi-line tray candidates. |
Comments suppressed due to low confidence (1)
src/pages/settings.test.tsx:63
- In this test file,
defaultPropsdoesn’t include the newly requiredshowTrayIcon/onShowTrayIconChangeprops forSettingsPage. Even if the test runner doesn’t typecheck TS, leaving these out can cause runtime errors in future tests that interact with the new checkbox. AddshowTrayIcon: true(or false) and anonShowTrayIconChangemock todefaultProps.
const defaultProps = {
plugins: [{ id: "a", name: "Alpha", enabled: true, primaryCandidates: [], trayLines: [] }],
onReorder: vi.fn(),
onToggle: vi.fn(),
onTrayLineToggle: vi.fn(),
autoUpdateInterval: 15 as const,
onAutoUpdateIntervalChange: vi.fn(),
themeMode: "system" as const,
onThemeModeChange: vi.fn(),
displayMode: "used" as const,
onDisplayModeChange: vi.fn(),
resetTimerDisplayMode: "relative" as const,
onResetTimerDisplayModeChange: vi.fn(),
globalShortcut: null,
onGlobalShortcutChange: vi.fn(),
startOnLogin: false,
onStartOnLoginChange: vi.fn(),
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| let tooltipText: string | undefined | ||
| let gridCellsToRender: TrayGridCell[] = [] | ||
| let providerIconUrlToRender = pluginsMetaRef.current.find((plugin) => plugin.id === trayProviderId)?.iconUrl |
There was a problem hiding this comment.
providerIconUrlToRender is never reassigned, so it can be a const instead of let. This avoids implying mutation and makes the render path easier to reason about.
| let providerIconUrlToRender = pluginsMetaRef.current.find((plugin) => plugin.id === trayProviderId)?.iconUrl | |
| const providerIconUrlToRender = pluginsMetaRef.current.find((plugin) => plugin.id === trayProviderId)?.iconUrl |
| export async function loadPluginSettings(): Promise<PluginSettings> { | ||
| const stored = await store.get<PluginSettings>(PLUGIN_SETTINGS_KEY); | ||
| if (!stored) return { ...DEFAULT_PLUGIN_SETTINGS }; | ||
| return { | ||
| order: Array.isArray(stored.order) ? stored.order : [], | ||
| disabled: Array.isArray(stored.disabled) ? stored.disabled : [], | ||
| trayLines: stored.trayLines && typeof stored.trayLines === "object" ? stored.trayLines : {}, | ||
| }; |
There was a problem hiding this comment.
loadPluginSettings treats any truthy object as valid trayLines (including null and arrays) and doesn't validate that values are string arrays. Since this data comes from persistent storage, malformed values can later break tray line toggling (e.g., calling .filter on a non-array) or cause unexpected iteration in tray rendering. Consider validating trayLines as a plain object and normalizing each entry to string[] (or dropping invalid entries) during load.
| const trayLines = { ...(settings.trayLines ?? {}) }; | ||
| for (const key in trayLines) { | ||
| if (!knownSet.has(key)) { | ||
| delete trayLines[key]; | ||
| } |
There was a problem hiding this comment.
normalizePluginSettings currently only deletes trayLines entries for unknown plugin ids, but it doesn’t validate/normalize the per-plugin arrays (type, duplicates) or filter labels against the plugin’s current primaryCandidates. This can lead to Settings showing no boxes checked while the tray falls back to a different line, or to runtime errors if a stored value isn’t an array. Consider normalizing each trayLines[id] to a deduped string[] and filtering to pluginsMeta.primaryCandidates.
| const trayLines = { ...(settings.trayLines ?? {}) }; | |
| for (const key in trayLines) { | |
| if (!knownSet.has(key)) { | |
| delete trayLines[key]; | |
| } | |
| const trayLines: Record<string, string[]> = {}; | |
| const storedTrayLines = settings.trayLines ?? {}; | |
| for (const plugin of plugins) { | |
| const id = plugin.id; | |
| const raw = storedTrayLines[id]; | |
| if (!Array.isArray(raw)) { | |
| continue; | |
| } | |
| const allowedLabels = new Set<string>(plugin.primaryCandidates ?? []); | |
| const normalized: string[] = []; | |
| const seenLabels = new Set<string>(); | |
| for (const value of raw) { | |
| if (typeof value !== "string") continue; | |
| if (!allowedLabels.has(value)) continue; | |
| if (seenLabels.has(value)) continue; | |
| seenLabels.add(value); | |
| normalized.push(value); | |
| } | |
| if (normalized.length > 0) { | |
| trayLines[id] = normalized; | |
| } |
| const configuredLabels = pluginSettings.trayLines?.[id] || [] | ||
| const targetLabels = configuredLabels.length > 0 | ||
| ? configuredLabels | ||
| : (meta.primaryCandidates.length > 0 ? [meta.primaryCandidates[0]] : []) | ||
|
|
There was a problem hiding this comment.
configuredLabels is taken directly from persisted pluginSettings.trayLines?.[id] and assumed to be an array (.length and iteration). If stored settings are malformed, this can cause incorrect behavior or throw at runtime. Prefer guarding with Array.isArray(...) (and treating anything else as []) to make tray rendering resilient.
| .then(async (img) => { | ||
| await tray.setIcon(img) | ||
| await tray.setIconAsTemplate(true) | ||
| await setTrayTitle(percentText) | ||
| await setTrayTitle(null) // Disabling native Title clipping entirely | ||
| const maybeSetTooltip = | ||
| (tray as TrayIcon & { setTooltip?: (value: string | null) => Promise<void> }).setTooltip | ||
| if (typeof maybeSetTooltip === "function") { | ||
| // If tooltip is null, clear current tooltip. | ||
| await maybeSetTooltip.call(tray, tooltipText ?? null).catch((error) => { | ||
| console.error("Failed to update tray tooltip:", error) |
There was a problem hiding this comment.
setTrayTitle(null) bypasses the Tauri type definition by widening setTitle to accept null. If the underlying API doesn’t accept null, this will throw and break tray updates. Prefer clearing the title with an empty string (or only call setTitle with strings) and keep the types aligned, optionally wrapping in a try/catch if platform support varies.
Description
Add multi-model tray usage display support, including:
Related Issue
Type of Change
Testing
bun run buildand it succeededbun run testand all tests passbun tauri devAdditional note:
bun run test:coveragecurrently fails due per-file coverage thresholds in existing/related hook files.Screenshots
Shows: tray icon toggle, per-provider multi-model selection, and tray output with icon + Codex Session/Weekly remaining usage.
Antigravity example: icon hidden; three model values rendered as 2-column layout (two rows, third item at top-right column).
Checklist
mainbranchSummary by cubic
Show multiple model usage in the tray with per-provider line selection and a tray icon visibility setting. Switched to a compact text grid with tooltips; native tray title is removed for cleaner display.
New Features
Refactors
Written for commit 2c86f87. Summary will update on new commits.