feat(mobile): readable Agents + Cluster views at 390px wide#251
feat(mobile): readable Agents + Cluster views at 390px wide#251
Conversation
At 390px the agent name, status chip, and four icon action buttons all competed for a single flex row, so the name truncated to "0.2..." and the actions collided with the chip. Gated the single-row layout behind useIsMobile and dropped a four-row mobile layout when small: row 1: dot + emoji + name + status chip row 2: host (with server icon) + vectors count row 3: paused/disk-warn chips (conditional) row 4: action buttons at 44x44 for touch targets Toolbar gets min-w-0 on the left group and shrink-0 on the Deploy Agent button so the CTA never gets squeezed out.
The fixed w-72 aside next to the worker card forced the right-hand Hardware panel into a 30px column on a 390px viewport, wrapping "x86_64" across multiple lines and breaking "Intel(R) Core(TM)..." one character per line. Reuse MobileSplitView (already used by MessagesApp) so mobile shows the worker list and detail one pane at a time with iOS-style slide nav, while desktop keeps the side-by-side layout unchanged. Also: flex-wrap the worker detail header so "Open worker UI" button drops below the name when cramped; break-all on the URL; sr-only the "Sort by" label to buy back ~50px of toolbar width.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 10 minutes and 34 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThese changes implement mobile-responsive UI layouts for the Agents and Cluster applications by introducing mobile-specific card designs, state-driven split views, and adjusted toolbar layouts, accompanied by a new mobile test suite to verify mobile rendering behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
desktop/src/apps/AgentsApp.tsx (1)
138-138: Avoid runninguseIsMobile()inside everyAgentRow.Each row instance adds its own media-query subscription. With many agents, listener count and resize work scale linearly. Compute once in
AgentsAppand passisMobileas a prop.♻️ Suggested refactor
- function AgentRow({ ... }) { - const isMobile = useIsMobile(); + function AgentRow({ isMobile, ... }: { isMobile: boolean; ... }) {export function AgentsApp({ windowId: _windowId }: { windowId: string }) { + const isMobile = useIsMobile(); ... {agents.map((agent) => ( <AgentRow key={agent.name} + isMobile={isMobile} agent={agent} ... /> ))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@desktop/src/apps/AgentsApp.tsx` at line 138, The component currently calls useIsMobile() inside every AgentRow which creates a media-query subscription per row; instead compute isMobile once in AgentsApp using useIsMobile() (the existing const isMobile = useIsMobile() in AgentsApp) and pass that boolean down as a prop (e.g., <AgentRow isMobile={isMobile} ...>), then remove/replace any useIsMobile() calls inside AgentRow and use the passed prop (isMobile) for conditional rendering/behavior.desktop/src/apps/__tests__/AgentsApp.mobile.test.tsx (1)
71-96: Prefer restoring/stubbingfetchto avoid test cross-contamination.Directly assigning
global.fetchinbeforeEachworks here, but it’s safer to restore mocks after each test.🧪 Suggested hygiene update
-import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; beforeEach(() => { ... - global.fetch = vi.fn().mockImplementation((url: string) => { + vi.stubGlobal("fetch", vi.fn().mockImplementation((url: string) => { ... - }); + })); }); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@desktop/src/apps/__tests__/AgentsApp.mobile.test.tsx` around lines 71 - 96, The test directly assigns global.fetch in beforeEach which can leak into other tests; replace the assignment with vi.stubGlobal('fetch', vi.fn(...)) using the same mock implementation, and add an afterEach(() => vi.unstubAllGlobals()) to restore the global fetch; reference beforeEach, global.fetch, vi.fn, vi.stubGlobal, and vi.unstubAllGlobals so you can locate and update the test setup and teardown.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@desktop/src/apps/__tests__/AgentsApp.mobile.test.tsx`:
- Line 18: The mock availableKvQuantOptions is declared async, returning a
Promise, but production usage is synchronous and the app consumes it in a state
updater; change the mock for availableKvQuantOptions to be a synchronous
function that directly returns the options object (remove async and the implicit
Promise) so consumers receive the plain object instead of a Promise; update the
mock definition where availableKvQuantOptions is declared in the test to return
{ k: ["fp16"], v: ["fp16"], boundary: false, flat: ["fp16"] } synchronously.
In `@desktop/src/apps/ClusterApp.tsx`:
- Around line 672-677: The mobile Back handler clears selection via
setSelected(null) but the worker polling logic re-selects the first worker
whenever selected === null; fix by adding a short-lived user-driven suppression
flag (e.g., suppressAutoSelect state) and set it true in the MobileSplitView
onBack handler before calling setSelected(null), then update the polling/refresh
code that currently auto-selects the first worker (the routine that assigns
selected when selected === null) to skip auto-selection while suppressAutoSelect
is true; clear suppressAutoSelect when the user explicitly selects a worker or
after the next successful poll/timeout so normal auto-selection resumes.
---
Nitpick comments:
In `@desktop/src/apps/__tests__/AgentsApp.mobile.test.tsx`:
- Around line 71-96: The test directly assigns global.fetch in beforeEach which
can leak into other tests; replace the assignment with vi.stubGlobal('fetch',
vi.fn(...)) using the same mock implementation, and add an afterEach(() =>
vi.unstubAllGlobals()) to restore the global fetch; reference beforeEach,
global.fetch, vi.fn, vi.stubGlobal, and vi.unstubAllGlobals so you can locate
and update the test setup and teardown.
In `@desktop/src/apps/AgentsApp.tsx`:
- Line 138: The component currently calls useIsMobile() inside every AgentRow
which creates a media-query subscription per row; instead compute isMobile once
in AgentsApp using useIsMobile() (the existing const isMobile = useIsMobile() in
AgentsApp) and pass that boolean down as a prop (e.g., <AgentRow
isMobile={isMobile} ...>), then remove/replace any useIsMobile() calls inside
AgentRow and use the passed prop (isMobile) for conditional rendering/behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d3c3603d-0059-474b-8fb5-8db27b136c82
📒 Files selected for processing (3)
desktop/src/apps/AgentsApp.tsxdesktop/src/apps/ClusterApp.tsxdesktop/src/apps/__tests__/AgentsApp.mobile.test.tsx
| <MobileSplitView | ||
| listTitle="Cluster" | ||
| detailTitle={selectedWorker?.name ?? ""} | ||
| listWidth={288} | ||
| selectedId={selected} | ||
| onBack={() => setSelected(null)} |
There was a problem hiding this comment.
Back navigation is overridden by the next worker refresh.
Line 677 clears selection, but the polling path re-selects the first worker whenever selected === null (Lines 548-551). On mobile, users can get pushed back into detail view ~10s after pressing Back.
💡 Suggested fix
+ const [userClearedSelection, setUserClearedSelection] = useState(false);
const fetchWorkers = useCallback(async () => {
try {
const res = await fetch("/api/cluster/workers", { headers: { Accept: "application/json" } });
if (res.ok) {
const json = await res.json();
if (Array.isArray(json)) {
setWorkers(json as ClusterWorker[]);
setSelected((cur) => {
if (cur && json.some((w: ClusterWorker) => w.name === cur)) return cur;
+ if (cur === null && userClearedSelection) return null;
return json.length > 0 ? (json[0] as ClusterWorker).name : null;
});
}
}
} catch {
/* ignore */
}
setLoading(false);
- }, []);
+ }, [userClearedSelection]);
...
<MobileSplitView
...
- onBack={() => setSelected(null)}
+ onBack={() => {
+ setUserClearedSelection(true);
+ setSelected(null);
+ }}
list={
...
{sortedWorkers.map((w) => (
<WorkerListCard
key={w.name}
worker={w}
selected={selected === w.name}
- onSelect={() => setSelected(w.name)}
+ onSelect={() => {
+ setUserClearedSelection(false);
+ setSelected(w.name);
+ }}
/>
))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@desktop/src/apps/ClusterApp.tsx` around lines 672 - 677, The mobile Back
handler clears selection via setSelected(null) but the worker polling logic
re-selects the first worker whenever selected === null; fix by adding a
short-lived user-driven suppression flag (e.g., suppressAutoSelect state) and
set it true in the MobileSplitView onBack handler before calling
setSelected(null), then update the polling/refresh code that currently
auto-selects the first worker (the routine that assigns selected when selected
=== null) to skip auto-selection while suppressAutoSelect is true; clear
suppressAutoSelect when the user explicitly selects a worker or after the next
successful poll/timeout so normal auto-selection resumes.
Production availableKvQuantOptions() in lib/cluster.ts is synchronous; the async mock did not match the real signature.
When the user hits 'back' in the mobile split-view the next 10s poll re-selected json[0] and pushed them back into the detail pane. Add a userNavigatedBack ref. Set it true in onBack, clear it false when the user manually taps a WorkerListCard. fetchWorkers now skips auto-selecting the first worker while the ref is set.
…257) Previous PRs (#251 mobile polish, #254 desktop icons, #255 install-to- worker picker) committed only the TypeScript sources under desktop/src/ without rebuilding the vite output under static/desktop/. The Pi serves the compiled bundles directly, so the browser never loaded the new code and the mobile Agents/Cluster views, service icons, and worker picker all silently rendered the pre-polish implementation. Rebuild catches all three up in a single hashed-asset turn.
Summary
Two taOS mobile-PWA views were unreadable on narrow screens. Fixed without changing desktop layout.
Agents view
The agent row packed the ID, status chip, and four icon actions into a single flex line — at 390px the name collapsed to "0.2..." and the actions fought the chip. Gated the single-row layout behind
useIsMobile; on mobile the row now stacks:Toolbar:
min-w-0on the left group andshrink-0on Deploy Agent so the CTA never gets squeezed out.Cluster view
The fixed
w-72aside next to the worker card forced the right-hand Hardware panel into a ~30px column on a 390px viewport, wrapping "x86_64" and breaking the CPU model one character per line.Replaced the split with
MobileSplitView(already used byMessagesApp). Mobile gets iOS-style one-pane-at-a-time slide nav; desktop keeps side-by-side layout. Worker detail header also flex-wraps now so "Open worker UI" drops below the name when cramped, URL usesbreak-all, and the "Sort by" label issr-onlyto reclaim width.Changes
desktop/src/apps/AgentsApp.tsx— mobile stacked row + toolbar flex fix.desktop/src/apps/ClusterApp.tsx— MobileSplitView, wrapping header, sr-only label.desktop/src/apps/__tests__/AgentsApp.mobile.test.tsx— 2 mobile-layout invariants.Style reference
Matched
MessagesAppconventions:useIsMobilehook,MobileSplitViewcomponent,shrink-0/min-w-0flex discipline,text-shell-text-*tokens. No new design primitives.Test plan
aria-label.Summary by CodeRabbit
Style
Tests