Redesign desktop UI/UX with sidebar shell and guided run flow#13
Conversation
- Replace top-bar with an app sidebar (branded logo, icon nav, engine status pill) for a modern desktop-app layout. - Add a full design system in styles.css: refined dark palette, spacing and radius scale, polished inputs/selects/switches, buttons, badges, cards, empty/loading states, and responsive behaviour. - Rework the New Run view to explain the actor-critic loop (how-it-works steps), add example goal chips, model-provider presets, autocompleted actor/critic runner ids, and toggle/stepper controls behind a friendly Advanced JSON editor. - Refresh Monitor, Results, Sessions and Secrets views with page headers, status pills, count tiles, empty states and clearer copy. Co-authored-by: Inzimam Ul Haq <27832433+inhaq@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe desktop app is redesigned from a topbar layout to a sidebar app shell. ChangesDesktop UI redesign
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/src/main.ts`:
- Around line 765-779: The refresh function is not handling errors from
listSecretKeys() and deleteSecret() calls, causing unhandled rejections instead
of displaying errors to the user. Wrap the await listSecretKeys() call in a
try-catch block to catch load errors and surface them via formError, similar to
how renderSessions() handles errors. Additionally, wrap the await
deleteSecret(k) and await refresh() calls in the delete button's click event
listener in a try-catch block to catch deletion errors and also surface them via
formError rather than letting the promise rejection propagate.
- Around line 181-239: The form controls goalArea, presetSelect, the runners
textarea, maxParallel, and the switch elements lack proper label associations,
making them inaccessible to assistive technology. Replace the adjacent text node
labels (currently wrapped in divs with class "label") with proper HTML label
elements that use the for attribute, and add corresponding id attributes to each
form control. For example, the h("div", { class: "label" }, ["Goal"]) should
become h("label", { for: "goal-input" }, ["Goal"]), and the goalArea textarea
should have an id="goal-input" attribute added. Apply this same pattern to all
other mentioned controls (presetSelect, runners textarea, maxParallel, and both
switches) throughout the form, ensuring each control has a unique id and a
corresponding label element with a matching for attribute.
In `@desktop/src/styles.css`:
- Around line 614-626: The `.nav-item span { display: none; }` rule in the
mobile breakpoint completely removes navigation labels from the accessibility
tree, making buttons inaccessible to screen readers and voice control users.
Replace the `display: none` approach with a technique that visually hides the
text while keeping it accessible, such as using a visually-hidden class with
clip-based hiding (position: absolute, clip: rect, overflow: hidden), or
alternatively add explicit `aria-label` attributes to the corresponding nav
buttons in desktop/index.html to provide accessible labels for screen readers.
- Around line 51-52: The CSS custom properties --mono and --sans use unquoted
font-family identifiers that violate the value-keyword-case Stylelint rule.
Quote all unquoted font family names (such as ui-monospace, Menlo, Consolas,
-apple-system, BlinkMacSystemFont, Roboto, Inter, and system-ui) while leaving
generic keywords like monospace and sans-serif unquoted. Apply this same quoting
convention to the other font-family declarations that are flagged by the linter
at the other affected locations in the file.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 50bb419c-321a-419f-8ba3-f9afe459df13
📒 Files selected for processing (3)
desktop/index.htmldesktop/src/main.tsdesktop/src/styles.css
| --mono: ui-monospace, "SF Mono", "SFMono-Regular", "Menlo", "Consolas", monospace; | ||
| --sans: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Inter, system-ui, sans-serif; |
There was a problem hiding this comment.
These values will trip the current Stylelint rule.
The browser will accept them, but the current value-keyword-case check is already flagging these declarations. Quoting the font-family identifiers and lowercasing the keyword values will keep this stylesheet lint-clean.
Also applies to: 67-67, 439-439
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 52-52: Expected "BlinkMacSystemFont" to be "blinkmacsystemfont" (value-keyword-case)
(value-keyword-case)
[error] 52-52: Expected "Roboto" to be "roboto" (value-keyword-case)
(value-keyword-case)
[error] 52-52: Expected "Inter" to be "inter" (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/styles.css` around lines 51 - 52, The CSS custom properties
--mono and --sans use unquoted font-family identifiers that violate the
value-keyword-case Stylelint rule. Quote all unquoted font family names (such as
ui-monospace, Menlo, Consolas, -apple-system, BlinkMacSystemFont, Roboto, Inter,
and system-ui) while leaving generic keywords like monospace and sans-serif
unquoted. Apply this same quoting convention to the other font-family
declarations that are flagged by the linter at the other affected locations in
the file.
Source: Linters/SAST tools
… and nav aria-labels
This pull request was created by @kiro-agent on behalf of @inhaq 👻
Comment with /kiro fix to address specific feedback or /kiro all to address everything.
Learn about Kiro Web
Summary
A significant UI/UX overhaul of the Loopwright desktop app to feel like a polished, modern desktop application and to make it obvious what the app does and how to use it.
styles.css): refined dark palette, consistent spacing/radius scale, polished inputs/selects/iOS-style switches, buttons, status badges, cards, empty/loading states, custom scrollbars, and a responsive layout that collapses to a top bar on narrow widths.No engine/API changes — this is purely the Tauri/browser frontend. All existing behaviour (live SSE stream, cancel, restart-with-confirm, secret validation) is preserved.
Testing
npm run build(tsc --noEmit && vite build) passes cleanly.Notes / limitations
apiKeyEnv).npm run preview.Summary by CodeRabbit