polish(desktop): unify settings and list controls - #11075
Conversation
Failure-Class: none
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Failure-Class: none
Failure-Class: none
Failure-Class: none Co-authored-by: Cursor <cursoragent@cursor.com>
Group the sidebar by Capture / Ask Omi / Alerts / Account / System, focus Listening on capture controls, merge Shortcuts onto Ask Omi, and restore Assistants and Integrations under Advanced. Failure-Class: none Co-authored-by: Cursor <cursoragent@cursor.com>
Failure-Class: none Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Elevate Settings/Tasks/Conversations with Geist type, cooler charcoal surfaces, double-bezel cards, selection accents, and richer chrome — staying within INV-UI-1 (no purple). Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
scaledFont() is the app-wide default text primitive (chat transcript, onboarding, memory graph, everywhere) — silently swapping its font resolution to Geist/CoreText on every call was over-scoped for a Settings/Tasks/Conversations-only polish pass and is the likely cause of the intermittent ChatTranscriptGestureHarnessTests CI failure (extra per-render CoreText work tightening an already-timing-sensitive scroll-settle window). Revert scaledFont() to plain .system(), keeping the tracking/letter-spacing tightening that was actually scoped to the touched pages. Verified: full 477-suite local run green, isolated target test green x3. Also registers the four previously-unwired Theme files (OmiChrome/OmiColors/OmiFont/OmiToggleStyle) in settings-basic.yaml's e2e coverage manifest, fixing the desktop-e2e-flow-coverage failure. Co-authored-by: Cursor <cursoragent@cursor.com>
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for the thoughtful polish here — the Settings grouping, shared search field/button chrome, and preserved deep-link aliases are directionally good, and the desktop checks are green.
I found one blocker to fix before this should merge:
desktop/macos/Desktop/Sources/MainWindow/SettingsSidebar.swift: the search index still keepsScreen CaptureandAudio Recordingentries under.rewindwithrewind.screencapture/rewind.audiorecording, but this PR moves those controls to the Listening/General section asgeneral.screencapture/general.audiorecording. That means settings search can still show duplicate Rewind results for capture/audio and route users to Rewind with a highlight id that no longer exists on that page. Please remove those stale Rewind search entries or repoint them to the actual Listening controls, and ideally add/extend the settings automation test so moved settings search entries remain routable to rendered card ids.
Because this also changes the macOS Settings information architecture and visual chrome, a human maintainer should still do the final product/UX pass after the stale search routing is fixed.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
Screen Capture / Audio Recording live under Listening (general.*). Keep settings search from surfacing duplicate Rewind entries with dead highlight ids. Failure-Class: none Co-authored-by: Cursor <cursoragent@cursor.com>
|
@Git-on-my-level addressed the blocker on the current head:
|
Remove card-in-card weight the visual pass left behind — emoji tiles, badge backgrounds, and drop shadows on conversation rows read as Slack/Notion chrome rather than a calmer list. Flatten to plain glyphs, quieter capsule filter pills, and outline task-category icons with uppercase mono labels. Reintroduce Geist on page titles (Conversations/Settings/Tasks headers) via the scoped .geist()/.geistMono() modifiers this time, not the shared scaledFont() default — keeps the app-wide font resolution path untouched after the earlier CI regression. Verified: full 477-suite local Swift run green, settings-specific suites (SettingsSidebarItemLayoutTests, AutomationSettingsSectionTests, SettingsControlMetricsTests) green, swift-format clean, pr-preflight reports no invariants affected. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@Git-on-my-level gentle re-review ask — your Rewind search-routing blocker is fixed on the current head ( |
Resolved on current head: stale Rewind search entries were removed, Listening general.* ids are covered by AutomationSettingsSectionTests, and current desktop checks are green.
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for the follow-up — I rechecked the current head.
The previous blocker around Settings search routing looks resolved: Screen Capture / Audio Recording are now indexed only under Listening with the general.* setting ids, the stale rewind.* search hits are gone, and the new automation test covers that regression. The relevant desktop checks are green as well.
I’m not formally approving because this still changes the macOS Settings information architecture and shared list/control chrome, so I’d like a human maintainer to make the final product/UX call. From the automation side, this is now a positive signal rather than a code blocker.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
|
@Git-on-my-level need human response Author-side is clear on
Merge gate is still |
Resolve desktop-swift-mainwindow.json line-count baseline to actual LOC after merge (TasksPage 6573; atlas/layout match main). Co-authored-by: Cursor <cursoragent@cursor.com>
|
Resolved merge conflict against current
Still needs human product/UX APPROVE for Settings IA + shared chrome (automation side already positive). |
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for resolving the merge conflict and refreshing this against current main.
I rechecked the current head (082f0ccea): the previous Settings search-routing blocker remains fixed — Screen Capture and Audio Recording are only indexed under Listening with the general.* ids, the stale rewind.* hits are gone, and the regression test is still present. The workflow-relevant baseline update matches the current diff, and the desktop checks reported for this head are green.
I’m keeping this as a positive automation signal rather than a formal approval because the PR still changes the macOS Settings information architecture and shared list/control chrome. A human maintainer should make the final product/UX call before merge.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
|
@Git-on-my-level need human response Thanks for the re-confirm on Author-side is clear. Merge is still blocked solely on a write-access / human product+UX APPROVE for the Settings IA + shared list/control chrome. Could a human maintainer make that final call when you have a moment? |
|
Still the macOS polish David assigned. CI green + MERGEABLE — looking for human UX APPROVE when someone can click through Settings / Tasks / Conversations. |
|
@kodjima33 gentle ask for human product/UX APPROVE when you have a moment. Author-side clear on
This is the macOS polish / taste PR. Happy to adjust anything you see in Settings / Tasks / Conversations. |
Summary
Polishes the macOS Settings, Tasks, and Conversations surfaces without changing
navigation, persistence, or task/conversation behavior.
Changes
(Capture, Ask Omi, Alerts & Privacy, Account, System): Listening-focused capture
controls, Shortcuts merged onto Ask Omi, and Assistants/Integrations restored
under clearly labeled Advanced categories.
OmiSearchFieldin Tasks so search treatment matchesConversations.
secondary, and destructive button styles.
shared compact control vocabulary; kept list toolbars usable at minimum width.
Before / After
Side-by-side comparisons (left = pre-polish baseline, right = this PR). Captured from separately named app bundles via the automation bridge.
Settings sidebar / Listening
Ask Omi (Floating Bar + Shortcuts merged)
Advanced
Tasks
Conversations
Visual craft pass
Cool monochrome structural polish on top of the IA reorg — Geist type, cooler charcoal surfaces, double-bezel cards, sidebar selection accents, and richer search/button chrome (still INV-UI-1: no purple).
Settings
Advanced
Tasks
Conversations
Verification
xcrun swift test --package-path Desktop --filter 'SettingsSidebarItemLayoutTests|AutomationSettingsSectionTests|SettingsControlMetricsTests'xcrun swift build -c debug --package-path DesktopOMI_APP_NAME="omi-ui-polish" ./run.sh --yolo --full --no-waitscreens captured via the automation bridge for visual review.
Invariants
No product invariants are affected.
Failure-Class: none