feat(studio): add first-wave React operator console - #22
Open
YashasVM wants to merge 2 commits into
Open
Conversation
📝 WalkthroughWalkthroughThe PR adds a Vite React desktop studio application. It includes deterministic studio state, operator routes, theme support, responsive behavior, reusable UI components, testing, and project tooling. ChangesDesktop studio application
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant App
participant DeterministicStudioAdapter
Operator->>App: select a route or trigger an action
App->>DeterministicStudioAdapter: update studio state
DeterministicStudioAdapter-->>App: publish StudioState
App-->>Operator: render feeds, statuses, alerts, and controls
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (7)
desktop/package.json (1)
10-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExpand the format script to cover the project.
npm run formatonly formats TypeScript and TSX files. It skips the HTML, Markdown, JavaScript, JSON, CSS, and package metadata added by this project. Useprettier --write .so.prettierignorecontrols exclusions, or include those extensions explicitly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/package.json` at line 10, Update the package format script to run Prettier across the project, preferably using `prettier --write .` so `.prettierignore` controls exclusions, instead of limiting formatting to TypeScript and TSX files.desktop/src/App.tsx (1)
24-24: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPrefer
useStateoveruseMemofor the adapter's stable identity.
useMemo(()=>provided??new DeterministicStudioAdapter(),[provided])relies on React preserving the memoized value across renders. React's own documentation treatsuseMemoas a performance optimization only, not a semantic guarantee — a future React version (or a currently valid edge case) could discard and recompute the memoized value, which would silently create a brand-newDeterministicStudioAdapterand reset all studio state (recording status, alerts, camera controls) without any user action.
useState(() => ...)'s lazy initializer is guaranteed to run only once per component instance, making it the correct primitive for holding a stateful object identity like this adapter.♻️ Proposed refactor
-export function App({adapter:provided}:{adapter?:StudioAdapter}){const adapter=useMemo(()=>provided??new DeterministicStudioAdapter(),[provided]);const state=useStudio(adapter); +export function App({adapter:provided}:{adapter?:StudioAdapter}){const [fallbackAdapter]=useState(()=>new DeterministicStudioAdapter());const adapter=provided??fallbackAdapter;const state=useStudio(adapter);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/App.tsx` at line 24, Replace the adapter initialization in App with a lazy useState initializer so the internally created DeterministicStudioAdapter retains a stable identity for the component instance. Preserve the provided adapter behavior and ensure the existing useStudio, keyboard handler, reset, and child-component flows continue using that state-held adapter.desktop/src/studio-adapter.test.ts (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard
vi.useRealTimers()against assertion failures.If an
expectcall betweenvi.useFakeTimers()andvi.useRealTimers()throws,vi.useRealTimers()never runs. Fake timers then stay active for later tests in this file, which can cause unrelated failures that are hard to diagnose.Use
beforeEach/afterEachto install and restore timers, or wrap the timer-dependent assertions intry/finally.♻️ Proposed refactor
- it("does not fake camera command confirmation",()=>{vi.useFakeTimers();const adapter=new DeterministicStudioAdapter();adapter.setCameraControl("cam-1","iso",400);expect(adapter.getSnapshot().cameras[0].controls.iso.state).toBe("pending");vi.advanceTimersByTime(350);expect(adapter.getSnapshot().cameras[0].controls.iso).toMatchObject({state:"confirmed",value:400});vi.useRealTimers()}) + it("does not fake camera command confirmation",()=>{ + vi.useFakeTimers() + try{ + const adapter=new DeterministicStudioAdapter();adapter.setCameraControl("cam-1","iso",400);expect(adapter.getSnapshot().cameras[0].controls.iso.state).toBe("pending");vi.advanceTimersByTime(350);expect(adapter.getSnapshot().cameras[0].controls.iso).toMatchObject({state:"confirmed",value:400}) + }finally{ + vi.useRealTimers() + } + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/studio-adapter.test.ts` at line 7, Update the timer setup in the test around “does not fake camera command confirmation” so real timers are always restored even when an assertion fails. Prefer moving fake-timer installation and cleanup into the test suite’s beforeEach/afterEach hooks, or wrap the existing timer-dependent assertions in try/finally while preserving the current assertions.desktop/src/studio-adapter.ts (1)
37-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid the
as CameraState[]cast on the hardcoded camera fixtures.The literal array at lines 37-42 mixes strings and numbers per row. TypeScript infers each row's element type as the union of every column's type, not as a per-position tuple, so destructured fields such as
health,thermal, andtallyare not checked against theHealthandCameraState["tally"]literal unions at this stage. Theas CameraState[]cast at line 45 then suppresses whatever narrower check remains. A typo in one of these string literals (for example"progrm"instead of"program") would not be caught by the type checker, even thoughtallyandthermalmatter for rendering logic inApp.tsx(Feed's className andheldFramehandling key off these exact literals).Type the raw tuple array explicitly instead of casting the final mapped result.
♻️ Proposed refactor
cameras: [ + ] as unknown as never, // placeholder removed below + cameras: ( + [ ["cam-1", "Camera 1", "Pixel 9 Pro", "ready", 94, 82, 78, "nominal", "program", "linear-gradient(135deg,`#17233a`,`#304b66` 52%,`#b06f46`)"], ["cam-2", "Camera 2", "Galaxy S25", "ready", 88, 96, 64, "warm", "preview", "linear-gradient(135deg,`#132c2c`,`#326a63` 55%,`#d39b68`)"], ["cam-3", "Camera 3", "Pixel 8", "warning", 62, 148, 42, "hot", "idle", "linear-gradient(135deg,`#321e2b`,`#75404f` 58%,`#d78771`)"], ["cam-4", "Camera 4", "Xperia 1 VI", "ready", 91, 74, 86, "nominal", "idle", "linear-gradient(135deg,`#242238`,`#514777` 55%,`#c29373`)"], - ].map(([id,name,model,health,signal,latencyMs,battery,thermal,tally,frame]) => ({ id,name,model,health,signal,latencyMs,battery,thermal,tally,frame, controls: { + ] as [string, string, string, Health, number, number, number, CameraState["thermal"], CameraState["tally"], string][] + ).map(([id,name,model,health,signal,latencyMs,battery,thermal,tally,frame]) => ({ id,name,model,health,signal,latencyMs,battery,thermal,tally,frame, controls: { lens: control(id === "cam-4" ? "24 mm" : "Main"), zoom: control(1), iso: control(200), shutter: control("1/60"), whiteBalance: control("4300 K"), focus: control("Auto"), torch: id === "cam-4" ? control(false,"unsupported","No torch on active lens") : control(false), stabilization: id === "cam-3" ? control("Standard","reconnect-required","Session rebuild required") : control("Standard") - }})) as CameraState[], + }})),(Remove the stray placeholder line; only one
cameras:initializer should remain, with the tuple type applied to the raw array and no cast on the mapped result.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/studio-adapter.ts` around lines 37 - 45, Replace the `as CameraState[]` cast in the `cameras` initializer with an explicit tuple type on the raw camera fixture array, using the corresponding literal unions and value types for each column. Keep the existing `.map` transformation and controls unchanged, and ensure the mapped result is accepted as `CameraState[]` through actual type checking so invalid `health`, `thermal`, or `tally` literals are reported.desktop/src/components/ui/sonner.tsx (1)
31-38: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winImport React before using
React.CSSProperties.Line 37 uses
React.CSSPropertiesas a type, but this file never imports React.chart.tsx,dialog.tsx, andtoggle-group.tsxall import React before using the identical pattern. Add an explicit import so the type resolves without relying on ambient/UMD global fallback behavior.🔧 Proposed fix to import the CSSProperties type explicitly
"use client" +import type { CSSProperties } from "react" import { useTheme } from "next-themes" import { Toaster as Sonner, type ToasterProps } from "sonner" import { CircleCheckIcon, InfoIcon, TriangleAlertIcon, OctagonXIcon, Loader2Icon } from "lucide-react"style={ { "--normal-bg": "var(--popover)", "--normal-text": "var(--popover-foreground)", "--normal-border": "var(--border)", "--border-radius": "var(--radius)", - } as React.CSSProperties + } as CSSProperties }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/components/ui/sonner.tsx` around lines 31 - 38, Add an explicit React import in the component containing the Sonner style object so the existing React.CSSProperties annotation resolves without relying on a global React namespace; keep the style values and type assertion unchanged.desktop/src/components/ui/sidebar.tsx (1)
86-86: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueSet an explicit
SameSiteattribute on the sidebar cookie.The cookie stores a UI preference only, so there is no data exposure. An explicit attribute removes browser console warnings about the missing value and states the intent.
♻️ Proposed change
- document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}` + document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}; SameSite=Lax`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/components/ui/sidebar.tsx` at line 86, Update the cookie assignment in the sidebar state persistence logic to include an explicit SameSite attribute, using the appropriate non-cross-site setting while preserving the existing cookie name, path, and max-age values.desktop/src/components/ui/input.tsx (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider typing
Inputwith the Base UI props type.
InputPrimitiveis imported from@base-ui/react/input, but this component usesReact.ComponentProps<"input">. That exposes only native input props and hides Base UI props such asrender,value, anddefaultValue. UseInputPrimitive.Propsif you want consumers to pass those props without a cast.♻️ Proposed type alignment
-function Input({ className, type, ...props }: React.ComponentProps<"input">) { +function Input({ className, type, ...props }: InputPrimitive.Props) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/components/ui/input.tsx` at line 6, Update the Input component’s props annotation to use InputPrimitive.Props instead of React.ComponentProps<"input">, preserving the existing className and type destructuring while exposing Base UI props such as render, value, and defaultValue.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@desktop/README.md`:
- Around line 1-3: Update desktop/README.md lines 1-3 to describe the desktop
operator console, its supported commands, and current destinations instead of
the generic Vite scaffold. Update desktop/index.html lines 5-7 to reference the
shipped product favicon and use the same product title, replacing vite.svg and
vite-app.
In `@desktop/src/App.tsx`:
- Line 20: Update the non-boolean control input in Cameras so numeric controls
such as zoom and iso convert e.target.value back to a number before calling
adapter.setCameraControl, while preserving string values for non-numeric
controls. Use the existing item.value type to determine the conversion and keep
boolean handling unchanged.
- Line 24: Update the keydown handler in App so its interactive-target guard
also excludes native button elements alongside input, textarea, and select.
Preserve the existing shortcut behavior for other targets while allowing focused
buttons to retain their native Space/Enter activation.
In `@desktop/src/components/theme-provider.tsx`:
- Around line 182-205: Update the storage event filter in the theme-provider
useEffect handler so it also processes events where event.key is null, which
represent localStorage.clear(). Preserve the existing storageArea check and
storageKey-specific handling, and reset the theme to defaultTheme when the clear
event removes the stored theme.
In `@desktop/src/components/ui/field.tsx`:
- Line 3: Update the imports in the field component so the React namespace is
available before any React.ComponentProps or similar namespace references,
either by adding the React namespace import or by replacing those references
with named imports from react.
In `@desktop/src/components/ui/scroll-area.tsx`:
- Around line 35-40: Update the scrollbar className in ScrollArea and the
corresponding orientation-dependent classes in Slider to use
data-[orientation=horizontal]: and data-[orientation=vertical]: selectors,
matching Base UI’s data-orientation attributes. Apply the same change in
desktop/src/components/ui/scroll-area.tsx lines 35-40 and
desktop/src/components/ui/slider.tsx lines 21-37.
In `@desktop/src/components/ui/sidebar.tsx`:
- Around line 152-206: Preserve and apply the dir prop on every Sidebar
rendering path: add it to the root div in the collapsible="none" branch and to
the desktop root element, while retaining it on SheetContent for mobile. In the
mobile branch, avoid relying on spreading div props onto Sheet (Dialog.Root);
forward applicable Sidebar props such as className and style to the rendered
SheetContent instead.
In `@desktop/src/components/ui/slider.tsx`:
- Around line 40-45: Update the slider wrapper’s generated thumbs in the
Array.from mapping to accept caller-provided per-thumb accessibility properties,
such as a thumbProps object or getAriaLabel callback, and apply the resulting
aria-label to each SliderPrimitive.Thumb. Ensure range sliders can provide
distinct names using the current thumb index, while preserving existing thumb
rendering and behavior.
- Around line 13-17: Update the _values calculation in the slider wrapper so
scalar value or defaultValue props produce a single thumb, while array props
preserve their existing thumb count; keep the [min, max] fallback only when
neither prop is provided.
In `@desktop/src/components/ui/sonner.tsx`:
- Around line 1-8: Update the useTheme import in the Toaster component to use
the application’s local hook from components/theme-provider.tsx via the
`@/components/theme-provider.tsx` alias, removing the direct next-themes
dependency while preserving the existing theme usage.
In `@desktop/src/components/ui/tabs.tsx`:
- Around line 6-21: Update the Tabs component’s TabsPrimitive.Root props to pass
the destructured orientation value directly as orientation={orientation}; remove
the manually set data-orientation attribute and preserve the existing styling
and prop forwarding.
In `@desktop/src/index.css`:
- Line 4: Update the `.status i` rule’s `background` declaration from
`currentColor` to the lowercase `currentcolor` keyword to satisfy Stylelint’s
keyword-case requirement.
- Line 4: Update the root theme styling used with ThemeProvider so the light
class on document.documentElement applies a complete light palette, including
overriding hard-coded dark backgrounds, borders, and text colors in selectors
such as body, .nav, .panel, inputs, and related surfaces. Ensure selecting light
visibly changes the interface while preserving the existing dark theme;
alternatively remove the light theme option if support cannot be implemented.
- Line 5: Update the collapsed sidebar route buttons rendered by the navigation
component to preserve accessible names when `.nav nav span` is hidden at the
1000px breakpoint. Add each route’s label, such as `r.label`, via an aria-label
or retain it in a visually hidden label, without changing the visual collapsed
layout.
In `@desktop/src/studio-adapter.ts`:
- Line 63: Update simulateProgramFailure to deduplicate the "program-failure"
alert before appending it to state.alerts: preserve the existing alert list when
an alert with that id already exists, while still adding the alert on the first
failure event.
- Around line 56-61: Replace the shared revision counter used by
setCameraControl with per-(cameraId, key) pending tokens, and have each timeout
validate only the token for its own control pair so unrelated commands do not
cancel it. Update reset to clear or invalidate all pending tokens, preventing
callbacks created before reset from mutating the reset state.
---
Nitpick comments:
In `@desktop/package.json`:
- Line 10: Update the package format script to run Prettier across the project,
preferably using `prettier --write .` so `.prettierignore` controls exclusions,
instead of limiting formatting to TypeScript and TSX files.
In `@desktop/src/App.tsx`:
- Line 24: Replace the adapter initialization in App with a lazy useState
initializer so the internally created DeterministicStudioAdapter retains a
stable identity for the component instance. Preserve the provided adapter
behavior and ensure the existing useStudio, keyboard handler, reset, and
child-component flows continue using that state-held adapter.
In `@desktop/src/components/ui/input.tsx`:
- Line 6: Update the Input component’s props annotation to use
InputPrimitive.Props instead of React.ComponentProps<"input">, preserving the
existing className and type destructuring while exposing Base UI props such as
render, value, and defaultValue.
In `@desktop/src/components/ui/sidebar.tsx`:
- Line 86: Update the cookie assignment in the sidebar state persistence logic
to include an explicit SameSite attribute, using the appropriate non-cross-site
setting while preserving the existing cookie name, path, and max-age values.
In `@desktop/src/components/ui/sonner.tsx`:
- Around line 31-38: Add an explicit React import in the component containing
the Sonner style object so the existing React.CSSProperties annotation resolves
without relying on a global React namespace; keep the style values and type
assertion unchanged.
In `@desktop/src/studio-adapter.test.ts`:
- Line 7: Update the timer setup in the test around “does not fake camera
command confirmation” so real timers are always restored even when an assertion
fails. Prefer moving fake-timer installation and cleanup into the test suite’s
beforeEach/afterEach hooks, or wrap the existing timer-dependent assertions in
try/finally while preserving the current assertions.
In `@desktop/src/studio-adapter.ts`:
- Around line 37-45: Replace the `as CameraState[]` cast in the `cameras`
initializer with an explicit tuple type on the raw camera fixture array, using
the corresponding literal unions and value types for each column. Keep the
existing `.map` transformation and controls unchanged, and ensure the mapped
result is accepted as `CameraState[]` through actual type checking so invalid
`health`, `thermal`, or `tally` literals are reported.
🪄 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: 9f64786e-d873-49de-91ad-c350c499a5b5
⛔ Files ignored due to path filters (3)
desktop/bun.lockis excluded by!**/*.lockdesktop/public/vite.svgis excluded by!**/*.svgdesktop/src/assets/react.svgis excluded by!**/*.svg
📒 Files selected for processing (49)
desktop/.gitignoredesktop/.prettierignoredesktop/.prettierrcdesktop/README.mddesktop/components.jsondesktop/eslint.config.jsdesktop/index.htmldesktop/package.jsondesktop/src/App.test.tsxdesktop/src/App.tsxdesktop/src/components/theme-provider.tsxdesktop/src/components/ui/alert-dialog.tsxdesktop/src/components/ui/avatar.tsxdesktop/src/components/ui/badge.tsxdesktop/src/components/ui/button.tsxdesktop/src/components/ui/card.tsxdesktop/src/components/ui/chart.tsxdesktop/src/components/ui/dialog.tsxdesktop/src/components/ui/dropdown-menu.tsxdesktop/src/components/ui/field.tsxdesktop/src/components/ui/input.tsxdesktop/src/components/ui/label.tsxdesktop/src/components/ui/progress.tsxdesktop/src/components/ui/scroll-area.tsxdesktop/src/components/ui/select.tsxdesktop/src/components/ui/separator.tsxdesktop/src/components/ui/sheet.tsxdesktop/src/components/ui/sidebar.tsxdesktop/src/components/ui/skeleton.tsxdesktop/src/components/ui/slider.tsxdesktop/src/components/ui/sonner.tsxdesktop/src/components/ui/switch.tsxdesktop/src/components/ui/table.tsxdesktop/src/components/ui/tabs.tsxdesktop/src/components/ui/textarea.tsxdesktop/src/components/ui/toggle-group.tsxdesktop/src/components/ui/toggle.tsxdesktop/src/components/ui/tooltip.tsxdesktop/src/hooks/use-mobile.tsdesktop/src/index.cssdesktop/src/lib/utils.tsdesktop/src/main.tsxdesktop/src/studio-adapter.test.tsdesktop/src/studio-adapter.tsdesktop/src/test-setup.tsdesktop/tsconfig.app.jsondesktop/tsconfig.jsondesktop/tsconfig.node.jsondesktop/vite.config.ts
Comment on lines
+1
to
+3
| # React + TypeScript + Vite + shadcn/ui | ||
|
|
||
| This is a template for a new Vite project with React, TypeScript, and shadcn/ui. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the remaining Vite scaffold identity.
The documentation and browser metadata still present this project as a generic Vite application. Update both locations so the operator console has one product name and one setup description.
desktop/README.md#L1-L3: Document the desktop operator console, supported commands, and current destinations.desktop/index.html#L5-L7: Replacevite.svgandvite-appwith the shipped product favicon and title.
📍 Affects 2 files
desktop/README.md#L1-L3(this comment)desktop/index.html#L5-L7
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/README.md` around lines 1 - 3, Update desktop/README.md lines 1-3 to
describe the desktop operator console, its supported commands, and current
destinations instead of the generic Vite scaffold. Update desktop/index.html
lines 5-7 to reference the shipped product favicon and use the same product
title, replacing vite.svg and vite-app.
| function Header({title,subtitle,actions}:{title:string;subtitle:string;actions?:React.ReactNode}){return <header className="page-header"><div><h1>{title}</h1><p>{subtitle}</p></div>{actions}</header>} | ||
| function Meter({level}:{level:number}){const pct=Math.max(4,Math.min(100,(level+60)*1.7));return <div className="meter" role="meter" aria-label={`Audio level ${level} decibels`} aria-valuenow={level} aria-valuemin={-60} aria-valuemax={0}><i style={{width:`${pct}%`}}/></div>} | ||
|
|
||
| function Cameras({state,adapter}:{state:ReturnType<typeof useStudio>;adapter:StudioAdapter}){const [selected,setSelected]=useState(state.cameras[0].id);const camera=state.cameras.find(c=>c.id===selected)!;const labels:Record<CameraControl,string>={lens:"Lens",zoom:"Zoom",iso:"ISO",shutter:"Shutter",whiteBalance:"White balance",focus:"Focus",torch:"Torch",stabilization:"Stabilization"};return <main><Header title="Cameras" subtitle="Connected phones and authoritative camera controls"/><div className="camera-layout"><section className="camera-grid" aria-label="Connected phones">{state.cameras.map(c=><button key={c.id} className={selected===c.id?"selected":""} onClick={()=>setSelected(c.id)}><Feed camera={c}/><span className="camera-meta"><b>{c.name}</b><small>{c.signal}% signal · {c.battery}% battery</small></span></button>)}</section><aside className="inspector"><header><div><h2>{camera.name}</h2><p>{camera.model}</p></div><Status value={camera.health}/></header><div className="control-list">{(Object.keys(camera.controls) as CameraControl[]).map(key=>{const item=camera.controls[key];return <label key={key}><span><b>{labels[key]}</b><em className={`command ${item.state}`}>{stateLabel[item.state]}</em></span>{typeof item.value==="boolean"?<input type="checkbox" checked={item.value} disabled={item.state==="unsupported"} onChange={e=>adapter.setCameraControl(camera.id,key,e.target.checked)}/>:<input aria-label={labels[key]} value={String(item.value)} disabled={item.state==="unsupported"} onChange={e=>adapter.setCameraControl(camera.id,key,e.target.value)}/>} {item.detail?<small>{item.detail}</small>:null}</label>})}</div></aside></div></main>} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Numeric camera controls silently become strings after an edit.
The text <input> for non-boolean controls sends e.target.value (always a string) to adapter.setCameraControl. For zoom and iso, the initial value is a number (control(1), control(200)), but any edit through this input replaces it with a string, permanently changing the stored type for that control. Nothing currently reads these values arithmetically, so this has a safe fallback today, but it works against the PR's stated goal of a typed adapter with well-defined command state.
Convert the value back to a number for controls that started as numeric.
🔧 Proposed fix
-{typeof item.value==="boolean"?<input type="checkbox" checked={item.value} disabled={item.state==="unsupported"} onChange={e=>adapter.setCameraControl(camera.id,key,e.target.checked)}/>:<input aria-label={labels[key]} value={String(item.value)} disabled={item.state==="unsupported"} onChange={e=>adapter.setCameraControl(camera.id,key,e.target.value)}/>}
+{typeof item.value==="boolean"?<input type="checkbox" checked={item.value} disabled={item.state==="unsupported"} onChange={e=>adapter.setCameraControl(camera.id,key,e.target.checked)}/>:<input aria-label={labels[key]} value={String(item.value)} disabled={item.state==="unsupported"} onChange={e=>adapter.setCameraControl(camera.id,key,typeof item.value==="number"?Number(e.target.value):e.target.value)}/>}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function Cameras({state,adapter}:{state:ReturnType<typeof useStudio>;adapter:StudioAdapter}){const [selected,setSelected]=useState(state.cameras[0].id);const camera=state.cameras.find(c=>c.id===selected)!;const labels:Record<CameraControl,string>={lens:"Lens",zoom:"Zoom",iso:"ISO",shutter:"Shutter",whiteBalance:"White balance",focus:"Focus",torch:"Torch",stabilization:"Stabilization"};return <main><Header title="Cameras" subtitle="Connected phones and authoritative camera controls"/><div className="camera-layout"><section className="camera-grid" aria-label="Connected phones">{state.cameras.map(c=><button key={c.id} className={selected===c.id?"selected":""} onClick={()=>setSelected(c.id)}><Feed camera={c}/><span className="camera-meta"><b>{c.name}</b><small>{c.signal}% signal · {c.battery}% battery</small></span></button>)}</section><aside className="inspector"><header><div><h2>{camera.name}</h2><p>{camera.model}</p></div><Status value={camera.health}/></header><div className="control-list">{(Object.keys(camera.controls) as CameraControl[]).map(key=>{const item=camera.controls[key];return <label key={key}><span><b>{labels[key]}</b><em className={`command ${item.state}`}>{stateLabel[item.state]}</em></span>{typeof item.value==="boolean"?<input type="checkbox" checked={item.value} disabled={item.state==="unsupported"} onChange={e=>adapter.setCameraControl(camera.id,key,e.target.checked)}/>:<input aria-label={labels[key]} value={String(item.value)} disabled={item.state==="unsupported"} onChange={e=>adapter.setCameraControl(camera.id,key,e.target.value)}/>} {item.detail?<small>{item.detail}</small>:null}</label>})}</div></aside></div></main>} | |
| function Cameras({state,adapter}:{state:ReturnType<typeof useStudio>;adapter:StudioAdapter}){const [selected,setSelected]=useState(state.cameras[0].id);const camera=state.cameras.find(c=>c.id===selected)!;const labels:Record<CameraControl,string>={lens:"Lens",zoom:"Zoom",iso:"ISO",shutter:"Shutter",whiteBalance:"White balance",focus:"Focus",torch:"Torch",stabilization:"Stabilization"};return <main><Header title="Cameras" subtitle="Connected phones and authoritative camera controls"/><div className="camera-layout"><section className="camera-grid" aria-label="Connected phones">{state.cameras.map(c=><button key={c.id} className={selected===c.id?"selected":""} onClick={()=>setSelected(c.id)}><Feed camera={c}/><span className="camera-meta"><b>{c.name}</b><small>{c.signal}% signal · {c.battery}% battery</small></span></button>)}</section><aside className="inspector"><header><div><h2>{camera.name}</h2><p>{camera.model}</p></div><Status value={camera.health}/></header><div className="control-list">{(Object.keys(camera.controls) as CameraControl[]).map(key=>{const item=camera.controls[key];return <label key={key}><span><b>{labels[key]}</b><em className={`command ${item.state}`}>{stateLabel[item.state]}</em></span>{typeof item.value==="boolean"?<input type="checkbox" checked={item.value} disabled={item.state==="unsupported"} onChange={e=>adapter.setCameraControl(camera.id,key,e.target.checked)}/>:<input aria-label={labels[key]} value={String(item.value)} disabled={item.state==="unsupported"} onChange={e=>adapter.setCameraControl(camera.id,key,typeof item.value==="number"?Number(e.target.value):e.target.value)}/>} {item.detail?<small>{item.detail}</small>:null}</label>})}</div></aside></div></main>} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/App.tsx` at line 20, Update the non-boolean control input in
Cameras so numeric controls such as zoom and iso convert e.target.value back to
a number before calling adapter.setCameraControl, while preserving string values
for non-numeric controls. Use the existing item.value type to determine the
conversion and keep boolean handling unchanged.
| function AudioPage({state,adapter}:{state:ReturnType<typeof useStudio>;adapter:StudioAdapter}){return <main><Header title="Audio" subtitle="Phone microphones stay independent until explicitly routed"/><div className="notice"><ShieldAlert/><span><b>No automatic mix</b><small>Select each source intentionally. Solo is monitoring-only.</small></span></div><section className="mixer" aria-label="Phone microphone mixer">{state.audio.map(a=><article key={a.id}><header><span><AudioLines/><b>{a.name}</b></span><Status value={a.health}/></header><Meter level={a.level}/><div className="db">{a.level} dBFS <span>{a.syncMs>0?"+":""}{a.syncMs} ms sync</span></div><label>Gain <input type="range" min="-24" max="12" value={a.gain} onChange={e=>adapter.setAudio(a.cameraId,{gain:Number(e.target.value)})}/><output>{a.gain} dB</output></label><div className="audio-buttons"><Button variant={a.muted?"destructive":"outline"} onClick={()=>adapter.setAudio(a.cameraId,{muted:!a.muted})}>Mute</Button><Button variant={a.solo?"default":"outline"} onClick={()=>adapter.setAudio(a.cameraId,{solo:!a.solo})}>Solo monitor</Button></div><label className="route"><input type="checkbox" checked={a.routed} onChange={e=>adapter.setAudio(a.cameraId,{routed:e.target.checked})}/><span><b>Route to Program</b><small>{a.routed?"Explicitly included":"Not included"}</small></span></label></article>)}</section></main>} | ||
| function Studio({state,adapter}:{state:ReturnType<typeof useStudio>;adapter:StudioAdapter}){const p=state.cameras.find(c=>c.id===state.programId)!;const pv=state.cameras.find(c=>c.id===state.previewId)!;return <main className="studio"><Header title="Studio" subtitle="Live multiview and engine-owned edit decisions" actions={<Button variant="outline" onClick={()=>adapter.simulateProgramFailure()}><CircleAlert/>Simulate live failure</Button>}/><AlertRail alerts={state.alerts.filter(a=>a.severity==="critical")} onGo={()=>{}}/><section className="program-row"><div><label>PROGRAM</label><Feed camera={p}/></div><div><label>PREVIEW</label><Feed camera={pv}/></div></section><section className="switcher"><div className="multiview">{state.cameras.map(c=><button key={c.id} aria-pressed={c.id===state.previewId} onClick={()=>adapter.selectPreview(c.id)}><Feed camera={c} compact/><span>{c.tally==="program"?"ON AIR":c.tally==="preview"?"NEXT":"SELECT"}</span></button>)}</div><div className="take"><Button size="lg" onClick={()=>adapter.take("cut")}>CUT <kbd>Space</kbd></Button><Button size="lg" variant="outline" onClick={()=>adapter.take("dissolve")}>DISSOLVE <kbd>D</kbd></Button></div></section><section className="studio-foot"><Panel title="Program audio"><div className="audio-summary">{state.audio.filter(a=>a.routed).map(a=><div key={a.id}><AudioLines/><b>{a.name}</b><Meter level={a.level}/></div>)}</div></Panel><Panel title="Outputs"><div className="output-lines"><span>ISO recording <Status value={state.output.recording?"ready":"offline"}/></span><span>Virtual camera <Status value={state.output.virtualCamera}/></span><span>OBS adapter <Status value={state.output.obs}/></span></div></Panel></section></main>} | ||
|
|
||
| export function App({adapter:provided}:{adapter?:StudioAdapter}){const adapter=useMemo(()=>provided??new DeterministicStudioAdapter(),[provided]);const state=useStudio(adapter);const [route,setRoute]=useState<Route>(()=>(location.hash.slice(2) as Route)||"dashboard");const navigate=(next:Route)=>{location.hash=`/${next}`;setRoute(next)};useEffect(()=>{const handler=(e:KeyboardEvent)=>{if(e.target instanceof Element&&e.target.matches("input,textarea,select"))return;if(e.key===" "){e.preventDefault();adapter.take("cut")}if(e.key.toLowerCase()==="d")adapter.take("dissolve");if(e.altKey&&/[1-4]/.test(e.key))navigate(routes[Number(e.key)-1].id)};window.addEventListener("keydown",handler);return()=>window.removeEventListener("keydown",handler)},[adapter]);return <div className="app-shell"><a className="skip" href="#content">Skip to content</a><aside className="nav"><div className="brand"><Radio/><span><b>OpenStream</b><small>STUDIO</small></span></div><nav>{routes.map(r=><button key={r.id} aria-current={route===r.id?"page":undefined} onClick={()=>navigate(r.id)}><r.icon/><span>{r.label}</span><kbd>Alt {routes.indexOf(r)+1}</kbd></button>)}</nav><div className="nav-foot"><Status value={state.readiness.engine} label="Engine connected"/><button onClick={()=>adapter.reset()}><RotateCcw/>Reset simulator</button><button><Settings2/>Settings</button></div></aside><div className="workspace"><div className="topbar"><span><Activity/> LIVE SESSION 01:42:18</span><span>4 phones · 1080p30</span><Status value={state.output.recording?"ready":"offline"} label={state.output.recording?"REC ISO":"Not recording"}/></div><div id="content">{route==="dashboard"?<Dashboard state={state} onGo={navigate}/>:route==="cameras"?<Cameras state={state} adapter={adapter}/>:route==="audio"?<AudioPage state={state} adapter={adapter}/>:<Studio state={state} adapter={adapter}/>}</div></div></div>} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching App.tsx / tests / package types:"
git ls-files | rg '(^|/)App\.tsx$|(\.test|\.spec)\.(ts|tsx|js|jsx)$|package\.json$' || true
echo
echo "desktop/src/App.tsx outline and relevant lines:"
if [ -f desktop/src/App.tsx ]; then
wc -l desktop/src/App.tsx
sed -n '1,140p' desktop/src/App.tsx
fi
echo
echo "Search for button/native keyboard tests and selector matches:"
rg -n 'keyboard|keydown|button|input,textarea,select|preventDefault|take\("cut"\)|Reset simulator|Simulate live failure' desktop -S || true
echo
echo "Check for button DOM elements in App/jsx snippets:"
rg -n '<button|button|Reset simulator|Simulate live failure|Cameras|Studio' desktop/src -S || trueRepository: YashasVM/OpenStream
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Button implementation:"
cat -n desktop/src/components/ui/button.tsx
echo
echo "Browser Space/Enter button activation semantics from MDN docs:"
python3 - <<'PY'
import json, urllib.request
urls=[
"https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/preventDefault",
"https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#accessibility",
"https://developer.mozilla.org/en-US/docs/Web/API/Element/click_event_vs_click_method",
]
for url in urls:
print(f"\n--- {url} ---")
req=urllib.request.Request(url, headers={"User-Agent":"CodeRabbit-verifier"})
with urllib.request.urlopen(req, timeout=10) as r:
text=r.read().decode("utf-8", errors="replace")
for needle in ["preventDefault", "Space", "Enter", "click"]:
idx=text.lower().find(needle.lower())
if idx>=0:
start=max(0, idx-800); end=min(len(text), idx+1200)
print(text[start:end])
print("...")
PYRepository: YashasVM/OpenStream
Length of output: 6264
🌐 Web query:
MDN KeyboardEvent preventDefault button Space Enter click activation
💡 Result:
When using native HTML button elements (button, input type="button", input type="submit"), browsers automatically handle Enter and Space keys to trigger a click event [1]. You do not need to call preventDefault or manually trigger clicks for these native controls unless you are overriding their standard behavior [2][1]. For non-native, custom-built interactive controls (such as a div or span with role="button"), you must manually implement keyboard accessibility to replicate native behavior [3][1]. Key Implementation Details: 1. Keyboard Support: Attach a keydown event listener to the custom element to detect when the user presses Enter or Space [3][1]. 2. Triggering Action: Inside the keydown listener, check for "Enter" or " " (Space). If triggered, call the element's.click method to programmatically fire the click event [4]. 3. Using preventDefault: - Space Key: You should call event.preventDefault for the Space key [1]. Without it, the browser's default behavior—scrolling the page—will often occur when the Space key is pressed [5][1]. - Enter Key: If your custom control is a native element that has its own default behavior (like an anchor tag used as a button), you must call event.preventDefault to stop the browser's default navigation [3][1]. If your control is a purely generic element (like a
), preventDefault is generally not required for Enter, though it is often safe to include [1]. Example Pattern: element.addEventListener('keydown', (event) => { if (event.key === 'Enter' || event.key === ' ') { event.preventDefault; // Stop scrolling (Space) or default nav (a tags) element.click; // Programmatically trigger your click handler } }); Always prefer native HTML buttons over custom elements whenever possible to avoid these accessibility issues entirely [2][1].
Citations:
- 1: https://govtnz.github.io/web-a11y-guidance/wct/buttons/make-a-button-accessible/buttons-keyboard-mouse-and-touch-accessibility.html
- 2: http://adrianroselli.com/2022/04/brief-note-on-buttons-enter-and-space.html
- 3: https://www.w3.org/WAI/GL/wiki/Making_actions_keyboard_accessible_by_using_keyboard_event_handlers_with_WAI-ARIA_controls
- 4: https://keepinguptodate.com/pages/2019/04/accessible-javascript-click-handlers/
- 5: https://developer.mozilla.org/en-US/docs/Web/API/Event/preventDefault
Exclude native buttons from the global Space-key shortcut guard.
In desktop/src/App.tsx, the keydown handler skips the Space/d/Alt+digit shortcuts only when the target matches input,textarea,select. The app contains native <button> elements, including nav actions, "Reset simulator", camera selectors, and "Simulate live failure". When a native button has focus and the user presses Space, this handler calls e.preventDefault() (adapter.take("cut")), which blocks the button’s native activation instead of letting the user reset or navigate. Add native interactive controls to the exclusion check so focused buttons retain their standard Space/Enter behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/App.tsx` at line 24, Update the keydown handler in App so its
interactive-target guard also excludes native button elements alongside input,
textarea, and select. Preserve the existing shortcut behavior for other targets
while allowing focused buttons to retain their native Space/Enter activation.
Comment on lines
+182
to
+205
| React.useEffect(() => { | ||
| const handleStorageChange = (event: StorageEvent) => { | ||
| if (event.storageArea !== localStorage) { | ||
| return | ||
| } | ||
|
|
||
| if (event.key !== storageKey) { | ||
| return | ||
| } | ||
|
|
||
| if (isTheme(event.newValue)) { | ||
| setThemeState(event.newValue) | ||
| return | ||
| } | ||
|
|
||
| setThemeState(defaultTheme) | ||
| } | ||
|
|
||
| window.addEventListener("storage", handleStorageChange) | ||
|
|
||
| return () => { | ||
| window.removeEventListener("storage", handleStorageChange) | ||
| } | ||
| }, [defaultTheme, storageKey]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -t f 'theme-provider*.test.*' desktop/src
rg -n -C 3 'StorageEvent|localStorage\.clear|storage.*event|ThemeProvider' desktop/src \
-g '*.{test,spec}.{ts,tsx}'Repository: YashasVM/OpenStream
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant theme provider file and nearby usages/tests.
fd -t f 'theme-provider' desktop/src || true
wc -l desktop/src/components/theme-provider.tsx
sed -n '150,220p' desktop/src/components/theme-provider.tsx
rg -n -C 3 'theme-provider|ThemeProvider|useTheme|storage:' desktop/src || trueRepository: YashasVM/OpenStream
Length of output: 6635
🌐 Web query:
MDN StorageEvent key null localStorage.clear DOMStorage
💡 Result:
When the localStorage.clear method is invoked, it triggers a storage event on all Window objects sharing the same storage area (except the one that initiated the clear) [1]. In this specific case, the StorageEvent object's key property is set to null [2][3]. According to the Web Storage specification, when the storage event is triggered by the clear method, the event's key, oldValue, and newValue attributes are all initialized to null [3][4]. This behavior serves as a signal to event listeners that the entire storage object has been cleared rather than a single item being modified or removed [2][5].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/Window/storage_event
- 2: https://developer.mozilla.org/en-US/docs/Web/API/StorageEvent
- 3: https://www.w3.org/TR/2016/REC-webstorage-20160419/
- 4: https://www.w3.org/TR/2015/CR-webstorage-20150609/
- 5: https://html.spec.whatwg.org/multipage/webstorage.html
Handle localStorage.clear() storage events.
localStorage.clear() can trigger a storage event with event.key === null. Since this handler ignores every event where event.key !== storageKey, ThemeContext keeps the old theme after another window clears the stored theme.
Proposed fix
- if (event.key !== storageKey) {
+ if (event.key !== storageKey && event.key !== null) {
return
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| React.useEffect(() => { | |
| const handleStorageChange = (event: StorageEvent) => { | |
| if (event.storageArea !== localStorage) { | |
| return | |
| } | |
| if (event.key !== storageKey) { | |
| return | |
| } | |
| if (isTheme(event.newValue)) { | |
| setThemeState(event.newValue) | |
| return | |
| } | |
| setThemeState(defaultTheme) | |
| } | |
| window.addEventListener("storage", handleStorageChange) | |
| return () => { | |
| window.removeEventListener("storage", handleStorageChange) | |
| } | |
| }, [defaultTheme, storageKey]) | |
| React.useEffect(() => { | |
| const handleStorageChange = (event: StorageEvent) => { | |
| if (event.storageArea !== localStorage) { | |
| return | |
| } | |
| if (event.key !== storageKey && event.key !== null) { | |
| return | |
| } | |
| if (isTheme(event.newValue)) { | |
| setThemeState(event.newValue) | |
| return | |
| } | |
| setThemeState(defaultTheme) | |
| } | |
| window.addEventListener("storage", handleStorageChange) | |
| return () => { | |
| window.removeEventListener("storage", handleStorageChange) | |
| } | |
| }, [defaultTheme, storageKey]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/components/theme-provider.tsx` around lines 182 - 205, Update the
storage event filter in the theme-provider useEffect handler so it also
processes events where event.key is null, which represent localStorage.clear().
Preserve the existing storageArea check and storageKey-specific handling, and
reset the theme to defaultTheme when the clear event removes the stored theme.
| @@ -0,0 +1,238 @@ | |||
| "use client" | |||
|
|
|||
| import { useMemo } from "react" | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '"allowUmdGlobalAccess"|"extends"' \
desktop/tsconfig.json desktop/tsconfig.app.json desktop/tsconfig.node.json
rg -n 'React\.(ComponentProps|ReactNode)' \
desktop/src/components/ui/field.tsxRepository: YashasVM/OpenStream
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Desktop tsconfig files:\n'
git ls-files desktop | grep -E '(^desktop/|/tsconfig\.)' || true
printf '\nSearch allowUmdGlobalAccess in repo:\n'
rg -n '"allowUmdGlobalAccess"\s*:' . || true
printf '\nSearch tsconfig files with relevant react/compiler options:\n'
for f in $(git ls-files | grep -E '(^desktop/|/tsconfig\.)'); do
if rg -q '"allowUmdGlobalAccess"\s*:|"jsx"\s*:|allowUmdGlobalAccess' "$f"; then
printf '\n--- %s ---\n' "$f"
rg -n '"jsx"\s*:"[^"]*"|"allowUmdGlobalAccess"\s*:\s*(true|false)|"strict"\s*:\s*(true|false)' "$f" || true
fi
done
printf '\nfield.tsx React usages and imports:\n'
if [ -f desktop/src/components/ui/field.tsx ]; then
cat -n desktop/src/components/ui/field.tsx | sed -n '1,80p'
fiRepository: YashasVM/OpenStream
Length of output: 4928
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in desktop/tsconfig.json desktop/tsconfig.app.json desktop/tsconfig.node.json; do
if [ -f "$f" ]; then
echo "--- $f ---"
cat -n "$f" | sed -n '1,160p'
fi
done
printf '\nVite config and package type deps:\n'
if [ -f desktop/vite.config.ts ]; then
cat -n desktop/vite.config.ts | sed -n '1,220p'
fi
jq '{type, dependencies, devDependencies, scripts, build} | {type, scripts, dependencies, "react_dts": (.dependencies["`@types/react`"] // .devDependencies["`@types/react`"] // null), "react": .dependencies.react, "vite": .dependencies.vite}' desktop/package.jsonRepository: YashasVM/OpenStream
Length of output: 3598
Import the React namespace before React.ComponentProps.
allowUmdGlobalAccess is not set, and this file is a module, so add import * as React or use member imports such as ComponentProps/ReactNode from react.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/components/ui/field.tsx` at line 3, Update the imports in the
field component so the React namespace is available before any
React.ComponentProps or similar namespace references, either by adding the React
namespace import or by replacing those references with named imports from react.
Comment on lines
+6
to
+21
| function Tabs({ | ||
| className, | ||
| orientation = "horizontal", | ||
| ...props | ||
| }: TabsPrimitive.Root.Props) { | ||
| return ( | ||
| <TabsPrimitive.Root | ||
| data-slot="tabs" | ||
| data-orientation={orientation} | ||
| className={cn( | ||
| "group/tabs flex gap-2 data-horizontal:flex-col", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Base UI React Tabs.Root orientation prop keyboard arrow navigation and data-orientation attribute
💡 Result:
In Base UI React, the Tabs.Root component includes an orientation prop that determines both the layout flow direction and the keyboard navigation behavior of the tabs [1][2][3]. Orientation Prop The orientation prop accepts either 'horizontal' (default) or 'vertical' [1][2][3]. Setting this prop is necessary to ensure that keyboard interactions align with the visual layout [4]: - Horizontal: Users navigate tabs using the left and right arrow keys [4]. - Vertical: Users navigate tabs using the up and down arrow keys [4]. Data-orientation Attribute The Tabs.Root component and its internal sub-components (such as Tabs.List, Tabs.Tab, and Tabs.Panel) expose a data-orientation attribute [1][2][5][3]. This attribute reflects the current orientation set on the root component, allowing for easy CSS styling based on the orientation state [3]. For example, you can target specific orientations in your CSS:.TabsList[data-orientation="vertical"] { flex-direction: column; } The attribute values will be either 'horizontal' or 'vertical', matching the orientation prop [1][6][3].
Citations:
- 1: https://base-ui.com/react/components/tabs.md
- 2: https://base-ui.com/react/components/tabs
- 3: https://mui-base-ui.mintlify.app/components/tabs
- 4: https://v6.mui.com/base-ui/react-tabs/
- 5: https://baseui-vue.com/docs/components/tabs
- 6: https://app.unpkg.com/@base-ui/react@1.6.0/files/tabs/root/TabsRootDataAttributes.mjs
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate tabs.tsx"
fd -a 'tabs\.tsx$' . | sed 's#^\./##'
echo
echo "Show desktop/src/components/ui/tabs.tsx around Tabs"
if [ -f desktop/src/components/ui/tabs.tsx ]; then
sed -n '1,80p' desktop/src/components/ui/tabs.tsx | nl -ba
fi
echo
echo "Search for TabsPrimitive import and Tabs usages"
rg -n "TabsPrimitive|<Tabs|orientation=\"vertical\"|orientation='vertical'|data-orientation|Groupdata.*tabs|data-horizontal" desktop/src || true
echo
echo "Package refs for `@base-ui` if present"
for f in package.json desktop/package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lockb; do
[ -f "$f" ] && { echo "--- $f ---"; rg -n "`@base-ui`|base-ui" "$f" | head -50 || true; }
doneRepository: YashasVM/OpenStream
Length of output: 321
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect component package files for Base UI version"
for f in package.json desktop/package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lockb; do
[ -f "$f" ] && { echo "--- $f ---"; rg -n "`@base-ui/react`|base-ui@|`@base-ui`|base-ui" "$f" | head -80 || true; }
done
echo
echo "Check node_modules source if already present"
if [ -d node_modules/@base-ui/react ]; then
fd . node_modules/@base-ui/react -t f | sed -n '1,80p'
rg -n "function TabsPrimitive|TabsPrimitive|data-orientation|orientation" node_modules/@base-ui/react -g '*.js' -g '*.mjs' -g '*.cjs' | head -120 || true
else
echo "node_modules/@base-ui/react not found"
fiRepository: YashasVM/OpenStream
Length of output: 357
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Show desktop/src/components/ui/tabs.tsx"
cat -n desktop/src/components/ui/tabs.tsx | sed -n '1,80p'
echo
echo "Search for TabsPrimitive and Tabs usages"
rg -n "TabsPrimitive|<Tabs|orientation=\"vertical\"|orientation='vertical'|data-orientation|group-data-vertical/tabs|group-data-horizontal/tabs" desktop/src || true
echo
echo "Inspect installed package metadata, if present"
if [ -d desktop/node_modules/@base-ui/react ]; then
sed -n '1,80p' desktop/node_modules/@base-ui/react/package.json
else
echo "desktop/node_modules/@base-ui/react not found"
fiRepository: YashasVM/OpenStream
Length of output: 7060
Forward orientation to TabsPrimitive.Root.
orientation is destructured from props, so the primitive never receives it. Base UI derives tab arrow-key navigation from the orientation prop on TabsPrimitive.Root; vertical tabs need orientation="vertical". With the current code, <Tabs orientation="vertical"> styles a vertical layout but keeps horizontal Left/Right navigation. Pass orientation={orientation} to TabsPrimitive.Root and remove the manual data-orientation attribute.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/components/ui/tabs.tsx` around lines 6 - 21, Update the Tabs
component’s TabsPrimitive.Root props to pass the destructured orientation value
directly as orientation={orientation}; remove the manually set data-orientation
attribute and preserve the existing styling and prop forwarding.
| @import "tailwindcss"; | ||
| @import "@fontsource-variable/geist"; | ||
|
|
||
| :root{font-family:"Geist Variable",system-ui,sans-serif;color:#edf2f5;background:#090d10;font-synthesis:none;--bg:#090d10;--surface:#10161a;--surface-2:#151d22;--line:#263139;--muted:#8e9aa2;--text:#edf2f5;--cyan:#5bd6df;--green:#63d29c;--amber:#f4ba62;--red:#ff6d71;--blue:#69a7ff}*{box-sizing:border-box}body{margin:0;min-width:320px;min-height:100vh;background:var(--bg);color:var(--text)}button,input{font:inherit}.app-shell{display:grid;grid-template-columns:220px minmax(0,1fr);min-height:100vh}.nav{position:sticky;top:0;height:100vh;border-right:1px solid var(--line);background:#0b1013;padding:20px 12px;display:flex;flex-direction:column;gap:30px}.brand{display:flex;align-items:center;gap:10px;padding:4px 10px}.brand>svg{color:var(--cyan)}.brand span{display:flex;flex-direction:column;line-height:1.05}.brand small{font-size:9px;letter-spacing:.24em;color:var(--muted);margin-top:4px}.nav nav{display:grid;gap:4px}.nav button{border:0;background:transparent;color:var(--muted);padding:10px 12px;border-radius:6px;display:flex;align-items:center;gap:10px;text-align:left;cursor:pointer}.nav button svg{width:17px}.nav button:hover,.nav button:focus-visible,.nav button[aria-current=page]{background:#182126;color:var(--text)}.nav button[aria-current=page]{box-shadow:inset 2px 0 var(--cyan)}.nav kbd{margin-left:auto;font-size:9px;opacity:0}.nav button:hover kbd,.nav button:focus kbd{opacity:.65}.nav-foot{margin-top:auto;display:grid;gap:6px}.workspace{min-width:0}.topbar{height:44px;border-bottom:1px solid var(--line);display:flex;align-items:center;gap:22px;padding:0 24px;font-size:11px;color:var(--muted);letter-spacing:.04em}.topbar>span{display:flex;align-items:center;gap:7px}.topbar>span:last-child{margin-left:auto}.topbar svg{width:14px;color:var(--red)}main{padding:26px;max-width:1600px;margin:auto}.page-header{display:flex;align-items:center;justify-content:space-between;gap:20px;margin-bottom:24px}.page-header h1{font-size:26px;line-height:1.1;letter-spacing:-.035em;margin:0}.page-header p{font-size:13px;color:var(--muted);margin:6px 0 0}.status{display:inline-flex;align-items:center;gap:6px;font-size:11px;white-space:nowrap}.status i{width:7px;height:7px;border-radius:50%;background:currentColor}.status-ready{color:var(--green)}.status-warning{color:var(--amber)}.status-offline{color:var(--red)}.readiness{display:grid;grid-template-columns:repeat(4,1fr);border:1px solid var(--line);border-radius:8px;overflow:hidden;margin-bottom:16px}.metric{border:0;border-right:1px solid var(--line);background:var(--surface);color:inherit;text-align:left;padding:15px 17px;display:grid;grid-template-columns:1fr auto;gap:9px;cursor:pointer}.metric:last-child{border:0}.metric>span:first-child{font-size:11px;color:var(--muted);text-transform:uppercase;letter-spacing:.07em}.metric strong{font-size:16px;grid-column:1/-1}.metric:hover{background:var(--surface-2)}.alert-rail{display:grid;gap:7px;margin:12px 0 16px}.alert{width:100%;border:1px solid #68512f;background:#211b12;color:var(--text);border-radius:6px;padding:10px 12px;display:flex;align-items:center;gap:10px;text-align:left;cursor:pointer}.alert.critical{border-color:#743438;background:#241113}.alert>svg{width:18px;color:var(--amber);flex:none}.alert.critical>svg{color:var(--red)}.alert>svg:last-child{margin-left:auto;color:var(--muted)}.alert span,.notice span{display:grid}.alert strong{font-size:12px}.alert small{font-size:11px;color:#c4b9a8;margin-top:2px}.two-col{display:grid;grid-template-columns:1.25fr 1fr;gap:16px;margin:16px 0}.panel{border:1px solid var(--line);border-radius:8px;background:var(--surface);overflow:hidden}.panel>header{height:48px;border-bottom:1px solid var(--line);display:flex;align-items:center;justify-content:space-between;padding:0 15px}.panel h2{font-size:13px;margin:0}.dual-feed,.program-row{display:grid;grid-template-columns:1fr 1fr;gap:10px;padding:12px}.dual-feed label,.program-row label{font-size:10px;letter-spacing:.14em;color:var(--muted);display:block;margin:0 0 6px}.feed{position:relative;aspect-ratio:16/9;border:1px solid #41505b;border-radius:5px;overflow:hidden;isolation:isolate;min-width:0}.feed.program{border:2px solid var(--red)}.feed.preview{border:2px solid var(--green)}.feed-grid{position:absolute;inset:0;background:linear-gradient(90deg,transparent 49.8%,#ffffff0b 50%,transparent 50.2%),linear-gradient(transparent 49.8%,#ffffff0b 50%,transparent 50.2%);background-size:33.333% 33.333%}.feed-top,.feed-bottom{position:absolute;z-index:1;left:0;right:0;display:flex;align-items:center;justify-content:space-between;padding:6px 8px;font-size:10px;background:linear-gradient(#080b0ddd,transparent)}.feed-top{top:0}.feed-bottom{bottom:0;background:linear-gradient(transparent,#080b0ddd);padding-top:20px}.feed.compact .feed-top .status,.feed.compact .feed-bottom{display:none}.held{position:absolute;inset:40% 0 auto;background:#1a090be8;border-block:1px solid var(--red);color:#fff;text-align:center;padding:7px;font-size:11px;font-weight:700;letter-spacing:.06em;display:flex;justify-content:center;align-items:center;gap:7px}.held svg{width:16px}.health-list{display:grid}.health-list button{color:inherit;background:transparent;border:0;border-bottom:1px solid var(--line);display:grid;grid-template-columns:75px 1fr auto;align-items:center;text-align:left;padding:11px 14px;cursor:pointer}.health-list button:hover{background:var(--surface-2)}.health-list span{display:grid}.health-list b{font-size:12px}.health-list small{color:var(--muted);font-size:10px;margin-top:2px}.health-list>button>span:last-child{color:var(--muted);font-size:11px}.audio-summary{padding:14px;display:grid;gap:10px}.audio-summary>div{display:grid;grid-template-columns:auto minmax(90px,1fr) minmax(100px,1fr);align-items:center;gap:10px}.audio-summary svg{width:17px;color:var(--cyan)}.audio-summary span{display:grid}.audio-summary b{font-size:12px}.audio-summary small{font-size:10px;color:var(--muted)}.meter{height:5px;background:#283138;border-radius:99px;overflow:hidden}.meter i{display:block;height:100%;background:linear-gradient(90deg,var(--cyan) 65%,var(--amber));border-radius:inherit}.output-grid{display:grid;grid-template-columns:repeat(3,1fr);padding:14px}.output-grid>div{display:grid;gap:6px;border-right:1px solid var(--line);padding:6px 14px}.output-grid>div:last-child{border:0}.output-grid svg{width:18px;color:var(--muted)}.output-grid span{font-size:10px;color:var(--muted)}.output-grid strong{font-size:18px}.camera-layout{display:grid;grid-template-columns:minmax(0,1fr) 340px;gap:18px}.camera-grid{display:grid;grid-template-columns:repeat(2,1fr);gap:12px;align-content:start}.camera-grid>button{padding:0;color:inherit;border:1px solid transparent;background:transparent;border-radius:7px;text-align:left;cursor:pointer}.camera-grid>button.selected{border-color:var(--cyan);box-shadow:0 0 0 2px #5bd6df24}.camera-meta{padding:10px;display:grid}.camera-meta b{font-size:12px}.camera-meta small{font-size:10px;color:var(--muted)}.inspector{border:1px solid var(--line);background:var(--surface);border-radius:8px;overflow:hidden;align-self:start;position:sticky;top:60px}.inspector>header{display:flex;justify-content:space-between;align-items:center;padding:15px;border-bottom:1px solid var(--line)}.inspector h2{font-size:15px;margin:0}.inspector p{font-size:11px;color:var(--muted);margin:3px 0 0}.control-list{display:grid}.control-list label{padding:12px 15px;border-bottom:1px solid var(--line);display:grid;grid-template-columns:1fr 115px;gap:7px;align-items:center}.control-list label>span{display:flex;align-items:center;gap:7px}.control-list b{font-size:11px}.control-list input:not([type=checkbox]){width:115px;background:#0d1215;border:1px solid #34414a;color:var(--text);border-radius:4px;padding:6px 8px}.control-list input[type=checkbox]{justify-self:end;width:18px;height:18px;accent-color:var(--cyan)}.control-list small{grid-column:1/-1;color:var(--muted);font-size:10px}.command{font-size:9px;font-style:normal;border:1px solid #3d4a51;color:var(--muted);border-radius:3px;padding:2px 4px}.command.pending{color:var(--amber)}.command.rejected,.command.conflict{color:var(--red)}.command.confirmed{color:var(--green)}.command.reconnect-required{color:var(--blue)}.notice{border:1px solid #41515a;background:#111a1f;border-radius:7px;padding:12px 14px;display:flex;gap:10px;align-items:center;margin-bottom:16px}.notice svg{color:var(--cyan)}.notice b{font-size:12px}.notice small{font-size:10px;color:var(--muted)}.mixer{display:grid;grid-template-columns:repeat(4,minmax(210px,1fr));gap:12px}.mixer article{border:1px solid var(--line);background:var(--surface);border-radius:8px;padding:14px;display:grid;gap:14px}.mixer header{display:flex;align-items:center;justify-content:space-between}.mixer header>span{display:flex;align-items:center;gap:7px}.mixer header svg{width:16px;color:var(--cyan)}.mixer b{font-size:12px}.db{font-size:20px;font-variant-numeric:tabular-nums}.db span{float:right;font-size:10px;color:var(--muted);margin-top:8px}.mixer article>label:not(.route){display:grid;grid-template-columns:1fr auto;gap:6px;font-size:10px;color:var(--muted)}.mixer input[type=range]{grid-column:1/-1;width:100%;accent-color:var(--cyan)}.audio-buttons{display:flex;gap:7px}.route{display:flex;align-items:center;gap:9px;border-top:1px solid var(--line);padding-top:12px}.route input{width:18px;height:18px;accent-color:var(--cyan)}.route span{display:grid}.route small{font-size:9px;color:var(--muted)}.studio{max-width:none}.program-row{padding:0;grid-template-columns:1fr 1fr}.switcher{display:grid;grid-template-columns:1fr auto;gap:16px;align-items:center;margin:14px 0}.multiview{display:grid;grid-template-columns:repeat(4,1fr);gap:8px}.multiview button{border:1px solid var(--line);background:var(--surface);padding:5px;color:inherit;cursor:pointer}.multiview button[aria-pressed=true]{border-color:var(--green)}.multiview button>span{font-size:9px;letter-spacing:.08em}.take{display:grid;gap:8px}.take button{min-width:160px;justify-content:space-between}.take kbd{font-size:9px;opacity:.6}.studio-foot{display:grid;grid-template-columns:1fr 1fr;gap:14px}.output-lines{display:grid;padding:10px 14px}.output-lines>span{display:flex;justify-content:space-between;border-bottom:1px solid var(--line);padding:9px 0;font-size:11px}.skip{position:fixed;left:10px;top:-50px;background:white;color:black;padding:8px;z-index:50}.skip:focus{top:10px}:focus-visible{outline:2px solid var(--cyan);outline-offset:2px} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the Stylelint keyword-case error.
Stylelint reports currentColor on line 4. Change background:currentColor to background:currentcolor.
🧰 Tools
🪛 Stylelint (17.14.1)
[error] 4-4: Expected "currentColor" to be "currentcolor" (value-keyword-case)
(value-keyword-case)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/index.css` at line 4, Update the `.status i` rule’s `background`
declaration from `currentColor` to the lowercase `currentcolor` keyword to
satisfy Stylelint’s keyword-case requirement.
Source: Linters/SAST tools
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Implement the light theme before exposing it.
ThemeProvider adds light or dark to document.documentElement, but this stylesheet has no theme-specific overrides. Selecting the light theme keeps the dark palette, including hard-coded dark surfaces. Add light styles for the variables and fixed-color selectors, or remove the light option until the stylesheet supports it.
🧰 Tools
🪛 Stylelint (17.14.1)
[error] 4-4: Expected "currentColor" to be "currentcolor" (value-keyword-case)
(value-keyword-case)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/index.css` at line 4, Update the root theme styling used with
ThemeProvider so the light class on document.documentElement applies a complete
light palette, including overriding hard-coded dark backgrounds, borders, and
text colors in selectors such as body, .nav, .panel, inputs, and related
surfaces. Ensure selecting light visibly changes the interface while preserving
the existing dark theme; alternatively remove the light theme option if support
cannot be implemented.
| @import "@fontsource-variable/geist"; | ||
|
|
||
| :root{font-family:"Geist Variable",system-ui,sans-serif;color:#edf2f5;background:#090d10;font-synthesis:none;--bg:#090d10;--surface:#10161a;--surface-2:#151d22;--line:#263139;--muted:#8e9aa2;--text:#edf2f5;--cyan:#5bd6df;--green:#63d29c;--amber:#f4ba62;--red:#ff6d71;--blue:#69a7ff}*{box-sizing:border-box}body{margin:0;min-width:320px;min-height:100vh;background:var(--bg);color:var(--text)}button,input{font:inherit}.app-shell{display:grid;grid-template-columns:220px minmax(0,1fr);min-height:100vh}.nav{position:sticky;top:0;height:100vh;border-right:1px solid var(--line);background:#0b1013;padding:20px 12px;display:flex;flex-direction:column;gap:30px}.brand{display:flex;align-items:center;gap:10px;padding:4px 10px}.brand>svg{color:var(--cyan)}.brand span{display:flex;flex-direction:column;line-height:1.05}.brand small{font-size:9px;letter-spacing:.24em;color:var(--muted);margin-top:4px}.nav nav{display:grid;gap:4px}.nav button{border:0;background:transparent;color:var(--muted);padding:10px 12px;border-radius:6px;display:flex;align-items:center;gap:10px;text-align:left;cursor:pointer}.nav button svg{width:17px}.nav button:hover,.nav button:focus-visible,.nav button[aria-current=page]{background:#182126;color:var(--text)}.nav button[aria-current=page]{box-shadow:inset 2px 0 var(--cyan)}.nav kbd{margin-left:auto;font-size:9px;opacity:0}.nav button:hover kbd,.nav button:focus kbd{opacity:.65}.nav-foot{margin-top:auto;display:grid;gap:6px}.workspace{min-width:0}.topbar{height:44px;border-bottom:1px solid var(--line);display:flex;align-items:center;gap:22px;padding:0 24px;font-size:11px;color:var(--muted);letter-spacing:.04em}.topbar>span{display:flex;align-items:center;gap:7px}.topbar>span:last-child{margin-left:auto}.topbar svg{width:14px;color:var(--red)}main{padding:26px;max-width:1600px;margin:auto}.page-header{display:flex;align-items:center;justify-content:space-between;gap:20px;margin-bottom:24px}.page-header h1{font-size:26px;line-height:1.1;letter-spacing:-.035em;margin:0}.page-header p{font-size:13px;color:var(--muted);margin:6px 0 0}.status{display:inline-flex;align-items:center;gap:6px;font-size:11px;white-space:nowrap}.status i{width:7px;height:7px;border-radius:50%;background:currentColor}.status-ready{color:var(--green)}.status-warning{color:var(--amber)}.status-offline{color:var(--red)}.readiness{display:grid;grid-template-columns:repeat(4,1fr);border:1px solid var(--line);border-radius:8px;overflow:hidden;margin-bottom:16px}.metric{border:0;border-right:1px solid var(--line);background:var(--surface);color:inherit;text-align:left;padding:15px 17px;display:grid;grid-template-columns:1fr auto;gap:9px;cursor:pointer}.metric:last-child{border:0}.metric>span:first-child{font-size:11px;color:var(--muted);text-transform:uppercase;letter-spacing:.07em}.metric strong{font-size:16px;grid-column:1/-1}.metric:hover{background:var(--surface-2)}.alert-rail{display:grid;gap:7px;margin:12px 0 16px}.alert{width:100%;border:1px solid #68512f;background:#211b12;color:var(--text);border-radius:6px;padding:10px 12px;display:flex;align-items:center;gap:10px;text-align:left;cursor:pointer}.alert.critical{border-color:#743438;background:#241113}.alert>svg{width:18px;color:var(--amber);flex:none}.alert.critical>svg{color:var(--red)}.alert>svg:last-child{margin-left:auto;color:var(--muted)}.alert span,.notice span{display:grid}.alert strong{font-size:12px}.alert small{font-size:11px;color:#c4b9a8;margin-top:2px}.two-col{display:grid;grid-template-columns:1.25fr 1fr;gap:16px;margin:16px 0}.panel{border:1px solid var(--line);border-radius:8px;background:var(--surface);overflow:hidden}.panel>header{height:48px;border-bottom:1px solid var(--line);display:flex;align-items:center;justify-content:space-between;padding:0 15px}.panel h2{font-size:13px;margin:0}.dual-feed,.program-row{display:grid;grid-template-columns:1fr 1fr;gap:10px;padding:12px}.dual-feed label,.program-row label{font-size:10px;letter-spacing:.14em;color:var(--muted);display:block;margin:0 0 6px}.feed{position:relative;aspect-ratio:16/9;border:1px solid #41505b;border-radius:5px;overflow:hidden;isolation:isolate;min-width:0}.feed.program{border:2px solid var(--red)}.feed.preview{border:2px solid var(--green)}.feed-grid{position:absolute;inset:0;background:linear-gradient(90deg,transparent 49.8%,#ffffff0b 50%,transparent 50.2%),linear-gradient(transparent 49.8%,#ffffff0b 50%,transparent 50.2%);background-size:33.333% 33.333%}.feed-top,.feed-bottom{position:absolute;z-index:1;left:0;right:0;display:flex;align-items:center;justify-content:space-between;padding:6px 8px;font-size:10px;background:linear-gradient(#080b0ddd,transparent)}.feed-top{top:0}.feed-bottom{bottom:0;background:linear-gradient(transparent,#080b0ddd);padding-top:20px}.feed.compact .feed-top .status,.feed.compact .feed-bottom{display:none}.held{position:absolute;inset:40% 0 auto;background:#1a090be8;border-block:1px solid var(--red);color:#fff;text-align:center;padding:7px;font-size:11px;font-weight:700;letter-spacing:.06em;display:flex;justify-content:center;align-items:center;gap:7px}.held svg{width:16px}.health-list{display:grid}.health-list button{color:inherit;background:transparent;border:0;border-bottom:1px solid var(--line);display:grid;grid-template-columns:75px 1fr auto;align-items:center;text-align:left;padding:11px 14px;cursor:pointer}.health-list button:hover{background:var(--surface-2)}.health-list span{display:grid}.health-list b{font-size:12px}.health-list small{color:var(--muted);font-size:10px;margin-top:2px}.health-list>button>span:last-child{color:var(--muted);font-size:11px}.audio-summary{padding:14px;display:grid;gap:10px}.audio-summary>div{display:grid;grid-template-columns:auto minmax(90px,1fr) minmax(100px,1fr);align-items:center;gap:10px}.audio-summary svg{width:17px;color:var(--cyan)}.audio-summary span{display:grid}.audio-summary b{font-size:12px}.audio-summary small{font-size:10px;color:var(--muted)}.meter{height:5px;background:#283138;border-radius:99px;overflow:hidden}.meter i{display:block;height:100%;background:linear-gradient(90deg,var(--cyan) 65%,var(--amber));border-radius:inherit}.output-grid{display:grid;grid-template-columns:repeat(3,1fr);padding:14px}.output-grid>div{display:grid;gap:6px;border-right:1px solid var(--line);padding:6px 14px}.output-grid>div:last-child{border:0}.output-grid svg{width:18px;color:var(--muted)}.output-grid span{font-size:10px;color:var(--muted)}.output-grid strong{font-size:18px}.camera-layout{display:grid;grid-template-columns:minmax(0,1fr) 340px;gap:18px}.camera-grid{display:grid;grid-template-columns:repeat(2,1fr);gap:12px;align-content:start}.camera-grid>button{padding:0;color:inherit;border:1px solid transparent;background:transparent;border-radius:7px;text-align:left;cursor:pointer}.camera-grid>button.selected{border-color:var(--cyan);box-shadow:0 0 0 2px #5bd6df24}.camera-meta{padding:10px;display:grid}.camera-meta b{font-size:12px}.camera-meta small{font-size:10px;color:var(--muted)}.inspector{border:1px solid var(--line);background:var(--surface);border-radius:8px;overflow:hidden;align-self:start;position:sticky;top:60px}.inspector>header{display:flex;justify-content:space-between;align-items:center;padding:15px;border-bottom:1px solid var(--line)}.inspector h2{font-size:15px;margin:0}.inspector p{font-size:11px;color:var(--muted);margin:3px 0 0}.control-list{display:grid}.control-list label{padding:12px 15px;border-bottom:1px solid var(--line);display:grid;grid-template-columns:1fr 115px;gap:7px;align-items:center}.control-list label>span{display:flex;align-items:center;gap:7px}.control-list b{font-size:11px}.control-list input:not([type=checkbox]){width:115px;background:#0d1215;border:1px solid #34414a;color:var(--text);border-radius:4px;padding:6px 8px}.control-list input[type=checkbox]{justify-self:end;width:18px;height:18px;accent-color:var(--cyan)}.control-list small{grid-column:1/-1;color:var(--muted);font-size:10px}.command{font-size:9px;font-style:normal;border:1px solid #3d4a51;color:var(--muted);border-radius:3px;padding:2px 4px}.command.pending{color:var(--amber)}.command.rejected,.command.conflict{color:var(--red)}.command.confirmed{color:var(--green)}.command.reconnect-required{color:var(--blue)}.notice{border:1px solid #41515a;background:#111a1f;border-radius:7px;padding:12px 14px;display:flex;gap:10px;align-items:center;margin-bottom:16px}.notice svg{color:var(--cyan)}.notice b{font-size:12px}.notice small{font-size:10px;color:var(--muted)}.mixer{display:grid;grid-template-columns:repeat(4,minmax(210px,1fr));gap:12px}.mixer article{border:1px solid var(--line);background:var(--surface);border-radius:8px;padding:14px;display:grid;gap:14px}.mixer header{display:flex;align-items:center;justify-content:space-between}.mixer header>span{display:flex;align-items:center;gap:7px}.mixer header svg{width:16px;color:var(--cyan)}.mixer b{font-size:12px}.db{font-size:20px;font-variant-numeric:tabular-nums}.db span{float:right;font-size:10px;color:var(--muted);margin-top:8px}.mixer article>label:not(.route){display:grid;grid-template-columns:1fr auto;gap:6px;font-size:10px;color:var(--muted)}.mixer input[type=range]{grid-column:1/-1;width:100%;accent-color:var(--cyan)}.audio-buttons{display:flex;gap:7px}.route{display:flex;align-items:center;gap:9px;border-top:1px solid var(--line);padding-top:12px}.route input{width:18px;height:18px;accent-color:var(--cyan)}.route span{display:grid}.route small{font-size:9px;color:var(--muted)}.studio{max-width:none}.program-row{padding:0;grid-template-columns:1fr 1fr}.switcher{display:grid;grid-template-columns:1fr auto;gap:16px;align-items:center;margin:14px 0}.multiview{display:grid;grid-template-columns:repeat(4,1fr);gap:8px}.multiview button{border:1px solid var(--line);background:var(--surface);padding:5px;color:inherit;cursor:pointer}.multiview button[aria-pressed=true]{border-color:var(--green)}.multiview button>span{font-size:9px;letter-spacing:.08em}.take{display:grid;gap:8px}.take button{min-width:160px;justify-content:space-between}.take kbd{font-size:9px;opacity:.6}.studio-foot{display:grid;grid-template-columns:1fr 1fr;gap:14px}.output-lines{display:grid;padding:10px 14px}.output-lines>span{display:flex;justify-content:space-between;border-bottom:1px solid var(--line);padding:9px 0;font-size:11px}.skip{position:fixed;left:10px;top:-50px;background:white;color:black;padding:8px;z-index:50}.skip:focus{top:10px}:focus-visible{outline:2px solid var(--cyan);outline-offset:2px} | ||
| @media(max-width:1000px){.app-shell{grid-template-columns:64px minmax(0,1fr)}.nav{padding-inline:8px}.brand span,.nav nav span,.nav kbd,.nav-foot span,.nav-foot button:last-child{display:none}.brand{justify-content:center}.nav button{justify-content:center}.nav-foot button{justify-content:center}.readiness{grid-template-columns:repeat(2,1fr)}.mixer{grid-template-columns:repeat(2,1fr)}.camera-layout{grid-template-columns:1fr}.inspector{position:static}.switcher{grid-template-columns:1fr}.take{grid-template-columns:1fr 1fr}.camera-grid{grid-template-columns:repeat(2,1fr)}} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve accessible names in the collapsed sidebar.
At the 1000px breakpoint, .nav nav span is hidden. The supplied desktop/src/App.tsx snippet places each route name only in that span and does not show an aria-label. The collapsed route buttons can become unnamed for screen readers. Add aria-label={r.label} or keep a visually hidden label.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/index.css` at line 5, Update the collapsed sidebar route buttons
rendered by the navigation component to preserve accessible names when `.nav nav
span` is hidden at the 1000px breakpoint. Add each route’s label, such as
`r.label`, via an aria-label or retain it in a visually hidden label, without
changing the visual collapsed layout.
Comment on lines
+56
to
+61
| setCameraControl(cameraId: string, key: CameraControl, value: string | number | boolean) { | ||
| const token=++this.revision; const update=(state:CommandState,detail?:string)=>this.emit({...this.state,cameras:this.state.cameras.map(c=>c.id===cameraId?{...c,controls:{...c.controls,[key]:control(value,state,detail)}}:c)}) | ||
| const current=this.state.cameras.find(c=>c.id===cameraId)?.controls[key]; if(current?.state==="unsupported"){ update("unsupported",current.detail); return } | ||
| update("pending","Awaiting authoritative phone state") | ||
| window.setTimeout(()=>{ if(token!==this.revision)return; update(key==="stabilization"&&cameraId==="cam-3"?"reconnect-required":"confirmed",key==="stabilization"&&cameraId==="cam-3"?"Apply & reconnect required":undefined)},350) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix the shared revision counter: it stalls unrelated pending commands.
this.revision is a single counter incremented for every setCameraControl call, on any camera, on any control. The stale-check if(token!==this.revision)return compares against this global counter, not against the specific (cameraId, key) pair being confirmed.
If a user adjusts one control, then adjusts a different control (different camera or different key) within 350 ms, the first command's window.setTimeout callback finds token !== this.revision and returns early. The first control stays stuck in "pending" state permanently, since nothing else re-triggers its confirmation. This breaks the "authoritative command states" behavior the PR introduces.
The same mechanism also lets a command issued right before reset() survive and mutate the freshly reset state, if no other command runs in between (reset() does not bump this.revision).
Track pending tokens per (cameraId, key) pair instead of one global counter, and clear them on reset.
🔧 Proposed fix
export class DeterministicStudioAdapter implements StudioAdapter {
- private state = initialState(); private listeners = new Set<() => void>(); private revision = 0
+ private state = initialState(); private listeners = new Set<() => void>(); private pending = new Map<string, number>()
subscribe = (listener: () => void) => { this.listeners.add(listener); return () => this.listeners.delete(listener) }
getSnapshot = () => this.state
private emit(state: StudioState) { this.state = state; this.listeners.forEach((listener) => listener()) }
selectPreview(cameraId: string) { this.emit({ ...this.state, previewId: cameraId, cameras: this.state.cameras.map(c => ({...c,tally:c.id===this.state.programId?"program":c.id===cameraId?"preview":"idle"})) }) }
take(transition: "cut" | "dissolve") { const oldProgram=this.state.programId; const next=this.state.previewId; this.emit({...this.state,transition,programId:next,previewId:oldProgram,cameras:this.state.cameras.map(c=>({...c,heldFrame:false,tally:c.id===next?"program":c.id===oldProgram?"preview":"idle"}))}) }
setCameraControl(cameraId: string, key: CameraControl, value: string | number | boolean) {
- const token=++this.revision; const update=(state:CommandState,detail?:string)=>this.emit({...this.state,cameras:this.state.cameras.map(c=>c.id===cameraId?{...c,controls:{...c.controls,[key]:control(value,state,detail)}}:c)})
+ const slot=`${cameraId}:${key}`; const token=(this.pending.get(slot)??0)+1; this.pending.set(slot,token)
+ const update=(state:CommandState,detail?:string)=>this.emit({...this.state,cameras:this.state.cameras.map(c=>c.id===cameraId?{...c,controls:{...c.controls,[key]:control(value,state,detail)}}:c)})
const current=this.state.cameras.find(c=>c.id===cameraId)?.controls[key]; if(current?.state==="unsupported"){ update("unsupported",current.detail); return }
update("pending","Awaiting authoritative phone state")
- window.setTimeout(()=>{ if(token!==this.revision)return; update(key==="stabilization"&&cameraId==="cam-3"?"reconnect-required":"confirmed",key==="stabilization"&&cameraId==="cam-3"?"Apply & reconnect required":undefined)},350)
+ window.setTimeout(()=>{ if(this.pending.get(slot)!==token)return; update(key==="stabilization"&&cameraId==="cam-3"?"reconnect-required":"confirmed",key==="stabilization"&&cameraId==="cam-3"?"Apply & reconnect required":undefined)},350)
}
setAudio(cameraId: string, patch: Partial<Pick<AudioSource,"routed"|"gain"|"muted"|"solo">>) { this.emit({...this.state,audio:this.state.audio.map(a=>a.cameraId===cameraId?{...a,...patch}:a)}) }
simulateProgramFailure() { /* unchanged */ }
- reset(){this.emit(initialState())}
+ reset(){this.pending.clear();this.emit(initialState())}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/studio-adapter.ts` around lines 56 - 61, Replace the shared
revision counter used by setCameraControl with per-(cameraId, key) pending
tokens, and have each timeout validate only the token for its own control pair
so unrelated commands do not cancel it. Update reset to clear or invalidate all
pending tokens, preventing callbacks created before reset from mutating the
reset state.
| window.setTimeout(()=>{ if(token!==this.revision)return; update(key==="stabilization"&&cameraId==="cam-3"?"reconnect-required":"confirmed",key==="stabilization"&&cameraId==="cam-3"?"Apply & reconnect required":undefined)},350) | ||
| } | ||
| setAudio(cameraId: string, patch: Partial<Pick<AudioSource,"routed"|"gain"|"muted"|"solo">>) { this.emit({...this.state,audio:this.state.audio.map(a=>a.cameraId===cameraId?{...a,...patch}:a)}) } | ||
| simulateProgramFailure() { const id=this.state.programId; this.emit({...this.state,cameras:this.state.cameras.map(c=>c.id===id?{...c,health:"offline",heldFrame:true}:c),alerts:[...this.state.alerts,{id:"program-failure",severity:"critical",title:"PROGRAM CAMERA OFFLINE — LAST FRAME HELD",detail:`${this.state.cameras.find(c=>c.id===id)?.name} disconnected. Other cameras and recording continue.`,destination:"/studio"}]}) } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Deduplicate the "program-failure" alert id before appending it.
simulateProgramFailure always appends an alert with the literal id "program-failure". The "Simulate live failure" button in desktop/src/App.tsx (Studio route) has no disabled state after the first click, so a second click appends a second alert entry with the same id into state.alerts. AlertRail in App.tsx renders these with key={a.id}, so two alerts with the same id produce duplicate React keys, which triggers React's duplicate-key warning and can cause incorrect reconciliation for the alert list.
Guard against re-adding the alert if it already exists, or derive a unique id per failure event.
🔧 Proposed fix
- simulateProgramFailure() { const id=this.state.programId; this.emit({...this.state,cameras:this.state.cameras.map(c=>c.id===id?{...c,health:"offline",heldFrame:true}:c),alerts:[...this.state.alerts,{id:"program-failure",severity:"critical",title:"PROGRAM CAMERA OFFLINE — LAST FRAME HELD",detail:`${this.state.cameras.find(c=>c.id===id)?.name} disconnected. Other cameras and recording continue.`,destination:"/studio"}]}) }
+ simulateProgramFailure() {
+ const id=this.state.programId
+ if(this.state.alerts.some(a=>a.id===`program-failure-${id}`))return
+ this.emit({...this.state,cameras:this.state.cameras.map(c=>c.id===id?{...c,health:"offline",heldFrame:true}:c),alerts:[...this.state.alerts,{id:`program-failure-${id}`,severity:"critical",title:"PROGRAM CAMERA OFFLINE — LAST FRAME HELD",detail:`${this.state.cameras.find(c=>c.id===id)?.name} disconnected. Other cameras and recording continue.`,destination:"/studio"}]})
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| simulateProgramFailure() { const id=this.state.programId; this.emit({...this.state,cameras:this.state.cameras.map(c=>c.id===id?{...c,health:"offline",heldFrame:true}:c),alerts:[...this.state.alerts,{id:"program-failure",severity:"critical",title:"PROGRAM CAMERA OFFLINE — LAST FRAME HELD",detail:`${this.state.cameras.find(c=>c.id===id)?.name} disconnected. Other cameras and recording continue.`,destination:"/studio"}]}) } | |
| simulateProgramFailure() { | |
| const id=this.state.programId | |
| if(this.state.alerts.some(a=>a.id===`program-failure-${id}`))return | |
| this.emit({...this.state,cameras:this.state.cameras.map(c=>c.id===id?{...c,health:"offline",heldFrame:true}:c),alerts:[...this.state.alerts,{id:`program-failure-${id}`,severity:"critical",title:"PROGRAM CAMERA OFFLINE — LAST FRAME HELD",detail:`${this.state.cameras.find(c=>c.id===id)?.name} disconnected. Other cameras and recording continue.`,destination:"/studio"}]}) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/studio-adapter.ts` at line 63, Update simulateProgramFailure to
deduplicate the "program-failure" alert before appending it to state.alerts:
preserve the existing alert list when an alert with that id already exists,
while still adding the alert on the first failure event.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
bun run typecheck✅bun run lint✅bun run test✅ 8 tests passingbun run build✅Rendered QA limitation
The in-app browser connection timed out. Playwright CLI was available, but the Chromium download also timed out, so no rendered screenshot/zoom evidence is claimed. Responsive CSS includes narrow layouts and 100/150/200% scaling-safe reflow; this remains the follow-up QA item.