feat(desktop): workspace number badges on modifier hold#3269
Open
tvrmsmith wants to merge 16 commits into
Open
feat(desktop): workspace number badges on modifier hold#3269tvrmsmith wants to merge 16 commits into
tvrmsmith wants to merge 16 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a toggleable feature to show workspace number badges (1–9) while a shortcut modifier is held: DB schema + migration, TRPC get/set, modifier-state store and listener hook, shortcut-parsing hook and tests, badge component, sidebar wiring, and settings UI. Changes
Sequence DiagramsequenceDiagram
participant User
participant Window as Browser/Window
participant Listener as useModifierKeyListener
participant Store as useModifierKeyStateStore
participant Hook as useWorkspaceShortcutModifiers
participant TRPC as electronTrpc.settings
participant UI as WorkspaceSidebar/UI
User->>Window: Hold modifier key (Cmd/Ctrl/Alt)
Window->>Listener: keydown event
Listener->>Store: pressKey(key)
Store->>Store: heldKeys updated, isModifierHeld = true
Store-->>Hook: state change triggers recompute
Hook->>TRPC: read cached getShowWorkspaceNumbersOnModifier
Hook-->>UI: provide shortcuts, combo→indices, shortcutLabels
UI->>UI: render WorkspaceShortcutBadge for matching indices
User->>Window: Release modifier key
Window->>Listener: keyup event
Listener->>Store: releaseKey(key)
Store->>Store: heldKeys updated, isModifierHeld = false
Store-->>Hook: state change triggers update
Hook-->>UI: badges hidden
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Greptile SummaryThis PR implements Arc-browser-style workspace shortcut badges: when the user holds the configured modifier key(s), numbered shortcut pills (e.g. Key changes:
Confidence Score: 4/5Safe to merge — feature is well-scoped, tests pass, and all issues found are non-blocking cleanup items. The implementation is architecturally sound: single listener mount, correct Zustand getState() usage after synchronous set(), thorough cleanup on unmount/blur, and consistent tRPC + optimistic-update patterns. The two P2 findings (dead hidden prop, per-item useMemo duplication) don't affect correctness or the primary user path. WorkspaceDiffStats.tsx (dead hidden prop) and WorkspaceListItem.tsx (useWorkspaceShortcutModifiers called per-instance).
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/stores/modifier-key-state.ts | New Zustand store tracking held modifier keys and a derived isModifierHeld boolean; clean immutable-set pattern with no issues. |
| apps/desktop/src/renderer/hooks/useModifierKeyListener.ts | Attaches keydown/keyup/blur listeners to compute modifier-held state; dependency array is correct, cleanup is thorough, Zustand getState() read after synchronous set() is safe. |
| apps/desktop/src/renderer/hooks/useWorkspaceShortcutModifiers.ts | Parses JUMP_TO_WORKSPACE_1-9 bindings into modifier key sets and display labels; void overrides trick to force memo recomputation on override changes is correct. |
| apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceDiffStats.tsx | Adds a hidden prop and conditional class, but the prop is never passed by WorkspaceListItem.tsx — dead code that should be removed. |
| apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx | Integrates badge display correctly — conditional rendering swaps badge for diff-stats on modifier hold; calls useWorkspaceShortcutModifiers() independently (N times for N items) when the result could be shared from a parent. |
| apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebar.tsx | Queries the setting and passes it to useModifierKeyListener — correct single mount-point for the global keyboard listener. |
| packages/local-db/src/schema/schema.ts | Adds showWorkspaceNumbersOnModifier boolean column to the settings table; consistent with existing column patterns for the local SQLite settings store. |
| apps/desktop/src/lib/trpc/routers/settings/index.ts | Adds get/set tRPC procedures for the new setting; uses onConflictDoUpdate upsert — consistent with other settings procedures. |
Sequence Diagram
sequenceDiagram
participant User
participant Window as window events
participant Listener as useModifierKeyListener
participant Store as ModifierKeyStateStore
participant Sidebar as WorkspaceSidebar
participant Item as WorkspaceListItem
participant Settings as tRPC settings
Sidebar->>Settings: getShowWorkspaceNumbersOnModifier
Settings-->>Sidebar: enabled (bool)
Sidebar->>Listener: useModifierKeyListener(enabled)
alt enabled = true
Listener->>Window: addEventListener(keydown/keyup/blur)
User->>Window: keydown (e.g. Meta)
Window->>Listener: handleKeyDown(Meta)
Listener->>Store: pressKey(Meta)
Listener->>Store: setState isModifierHeld true
Store-->>Item: isModifierHeld = true
Item->>Item: render WorkspaceShortcutBadge
User->>Window: keyup (Meta)
Window->>Listener: handleKeyUp(Meta)
Listener->>Store: releaseKey(Meta)
Listener->>Store: setState isModifierHeld false
Store-->>Item: isModifierHeld = false
Item->>Item: render WorkspaceDiffStats
else enabled = false
Listener->>Store: clearAll()
end
Comments Outside Diff (1)
-
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx, line 515 (link)useWorkspaceShortcutModifiersrecomputed in every list-item instanceEach
WorkspaceListItemindependently callsuseWorkspaceShortcutModifiers(), which runs auseMemocomputation. With, say, 20+ workspaces in the sidebar that's 20+ parallel memos all returning identical data whenever overrides or the component tree re-renders.The result is already computed in
ProjectSection(line 408) for the collapsed-header badge path. Consider computing it once at theWorkspaceSidebarlevel and passingshortcutLabels(aMap<number, string>) down as a prop or via React context, so the work is done once per render cycle instead of N times.
Reviews (1): Last reviewed commit: "fix(desktop): use dynamic shortcut label..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 16 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceShortcutBadge.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceShortcutBadge.tsx:4">
P3: JSDoc example is inaccurate — `label` receives the full formatted display string from `formatHotkeyDisplay` (e.g., `"⌘1"`, `"Ctrl+Shift+1"`), not a bare digit.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsx (2)
213-219: Consider using index-based keys to avoid potential collisions.Using
labelas the React key could cause issues if custom hotkey overrides result in duplicate labels (e.g., two workspaces bound to the same shortcut). While unlikely in practice, using the array index is safer here since the array is derived fresh each render.♻️ Suggested fix
{shortcutLabels.length > 0 && ( <div className="flex items-center gap-1 mt-1"> - {shortcutLabels.map((label) => ( - <WorkspaceShortcutBadge key={label} label={label} /> + {shortcutLabels.map((label, idx) => ( + <WorkspaceShortcutBadge key={idx} label={label} /> ))} </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsx` around lines 213 - 219, The current map over shortcutLabels in ProjectHeader uses the label string as the React key which can collide if labels duplicate; update the mapping that renders WorkspaceShortcutBadge to use the array index (e.g., map((label, idx) => <WorkspaceShortcutBadge key={idx} ... />)) so keys are stable per-render and avoid collisions while keeping the same label prop passed to WorkspaceShortcutBadge; locate the mapping over shortcutLabels in ProjectHeader (the shortcutLabels.map(...) block) and replace the key from label to the index variable.
317-323: Same key consideration applies here.For consistency, consider using index-based keys in this location as well.
♻️ Suggested fix
{shortcutLabels.length > 0 && ( <div className="flex items-center gap-1 ml-auto"> - {shortcutLabels.map((label) => ( - <WorkspaceShortcutBadge key={label} label={label} /> + {shortcutLabels.map((label, idx) => ( + <WorkspaceShortcutBadge key={idx} label={label} /> ))} </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsx` around lines 317 - 323, The mapped shortcut badges use the label string as the React key which can cause inconsistencies; update the map over shortcutLabels so the key uses the index (e.g., change shortcutLabels.map((label, i) => <WorkspaceShortcutBadge key={i} ... />)) to match the other usage and ensure stable keys; locate the shortcutLabels map and the WorkspaceShortcutBadge component invocation and replace the key from label to the index (or a deterministic label+index) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx`:
- Around line 403-446: The right-side controls currently swap the entire grid
when shortcut badges show, hiding the hover-close button; instead always render
the container grid and the hover actions div, and only conditionally render the
diff-stats vs. shortcut badge inside the stacked cell. Concretely, in
WorkspaceListItem.tsx keep the outer <div className="grid ..."> and the hover
actions <div className="flex ... group-hover:..."> always rendered, and change
the conditional to render either <WorkspaceDiffStats ... /> or
<WorkspaceShortcutBadge label={shortcutLabel} /> as the first child in that grid
(use the existing [&>*]:col-start-1 [&>*]:row-start-1 stacking), so
handleDeleteClick, isBranchWorkspace check, and the Tooltip/HotkeyLabel remain
present even when shortcutLabel is shown.
In `@packages/local-db/src/schema/schema.ts`:
- Around line 225-228: The schema adds showWorkspaceNumbersOnModifier via
integer("show_workspace_numbers_on_modifier", { mode: "boolean" }) but there is
no migration to add that column; create a new SQL migration that ALTERs the
settings table to add the show_workspace_numbers_on_modifier column with the
same boolean representation as the other settings (INTEGER mode, default 0/false
and NOT NULL or matching existing settings columns), and include it in the
drizzle migrations so upgraded DBs get the column before code uses
showWorkspaceNumbersOnModifier.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsx`:
- Around line 213-219: The current map over shortcutLabels in ProjectHeader uses
the label string as the React key which can collide if labels duplicate; update
the mapping that renders WorkspaceShortcutBadge to use the array index (e.g.,
map((label, idx) => <WorkspaceShortcutBadge key={idx} ... />)) so keys are
stable per-render and avoid collisions while keeping the same label prop passed
to WorkspaceShortcutBadge; locate the mapping over shortcutLabels in
ProjectHeader (the shortcutLabels.map(...) block) and replace the key from label
to the index variable.
- Around line 317-323: The mapped shortcut badges use the label string as the
React key which can cause inconsistencies; update the map over shortcutLabels so
the key uses the index (e.g., change shortcutLabels.map((label, i) =>
<WorkspaceShortcutBadge key={i} ... />)) to match the other usage and ensure
stable keys; locate the shortcutLabels map and the WorkspaceShortcutBadge
component invocation and replace the key from label to the index (or a
deterministic label+index) accordingly.
🪄 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
Run ID: 7950c5be-4e8d-4b19-a824-367f9880b7ae
📒 Files selected for processing (16)
apps/desktop/src/lib/trpc/routers/settings/index.tsapps/desktop/src/renderer/hooks/useModifierKeyListener.tsapps/desktop/src/renderer/hooks/useWorkspaceShortcutModifiers.test.tsapps/desktop/src/renderer/hooks/useWorkspaceShortcutModifiers.tsapps/desktop/src/renderer/routes/_authenticated/settings/behavior/components/BehaviorSettings/BehaviorSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceDiffStats.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceShortcutBadge.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/stores/modifier-key-state.test.tsapps/desktop/src/renderer/stores/modifier-key-state.tspackages/local-db/src/schema/schema.ts
Comment on lines
+403
to
+446
| {showShortcutBadge && shortcutLabel ? ( | ||
| <WorkspaceShortcutBadge | ||
| label={shortcutLabel} | ||
| className="shrink-0" | ||
| /> | ||
| ) : ( | ||
| <div className="grid shrink-0 h-5 [&>*]:col-start-1 [&>*]:row-start-1 items-center"> | ||
| {diffStats && ( | ||
| <WorkspaceDiffStats | ||
| additions={diffStats.additions} | ||
| deletions={diffStats.deletions} | ||
| isActive={isActive} | ||
| /> | ||
| )} | ||
| <div className="flex items-center justify-end gap-1.5 opacity-0 invisible group-hover:opacity-100 group-hover:visible transition-[opacity,visibility]"> | ||
| {shortcutLabel && ( | ||
| <WorkspaceShortcutBadge label={shortcutLabel} /> | ||
| )} | ||
| {!isBranchWorkspace && ( | ||
| <Tooltip delayDuration={300}> | ||
| <TooltipTrigger asChild> | ||
| <button | ||
| type="button" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| handleDeleteClick(); | ||
| }} | ||
| className="flex items-center justify-center text-muted-foreground hover:text-foreground" | ||
| aria-label="Close workspace" | ||
| > | ||
| <HiMiniXMark className="size-3.5" /> | ||
| </button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="top" sideOffset={4}> | ||
| <HotkeyLabel | ||
| label="Close workspace" | ||
| id={isActive ? "CLOSE_WORKSPACE" : undefined} | ||
| /> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| )} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| )} |
Contributor
There was a problem hiding this comment.
Keep the hover close action available while modifier badges are showing.
This branch swaps out the entire right-side controls area, not just the diff-stats slot. As a result, non-branch workspaces lose their hover close button whenever the modifier overlay is active. Only the diff-stats content should be replaced; the hover actions should stay rendered.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx`
around lines 403 - 446, The right-side controls currently swap the entire grid
when shortcut badges show, hiding the hover-close button; instead always render
the container grid and the hover actions div, and only conditionally render the
diff-stats vs. shortcut badge inside the stacked cell. Concretely, in
WorkspaceListItem.tsx keep the outer <div className="grid ..."> and the hover
actions <div className="flex ... group-hover:..."> always rendered, and change
the conditional to render either <WorkspaceDiffStats ... /> or
<WorkspaceShortcutBadge label={shortcutLabel} /> as the first child in that grid
(use the existing [&>*]:col-start-1 [&>*]:row-start-1 stacking), so
handleDeleteClick, isBranchWorkspace check, and the Tooltip/HotkeyLabel remain
present even when shortcutLabel is shown.
There was a problem hiding this comment.
I already saw this and thought it was acceptable to not show the 'X' on hover when the modifier is pressed because it added weird spacing to the worktree workspaces.
Contributor
There was a problem hiding this comment.
@TrevorSmith-Wellsky, that makes sense — the modifier-held state is momentary and losing the close button during that brief window is a reasonable tradeoff to avoid layout artifacts. I'll note this for future reviews.
(´• ω •`) ʕ•ᴥ•ʔ
✏️ Learnings added
Learnt from: TrevorSmith-Wellsky
Repo: superset-sh/superset PR: 3269
File: apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx:403-446
Timestamp: 2026-04-09T14:49:40.631Z
Learning: In `apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx`, when `showShortcutBadge && shortcutLabel` is true the entire right-side controls area (including the hover close button) is intentionally swapped out for the `WorkspaceShortcutBadge`. This is by design: keeping the close button visible alongside the badge caused weird spacing on worktree workspaces. Do not flag the missing hover-close button during modifier-held state as a bug.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: Jc7j
Repo: superset-sh/superset PR: 0
File: :0-0
Timestamp: 2026-03-11T20:46:55.809Z
Learning: The `DiffScrollIndicators` component (`apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/DiffScrollIndicators/DiffScrollIndicators.tsx`) uses inline `style` props throughout with dynamic hex constants. Hover states are handled via JS `onMouseEnter`/`onMouseLeave` handlers rather than CSS `:hover`, which is the correct approach here — mixing CSS `:hover` with inline styles would require `!important` overrides and be inconsistent with the rest of the component.
Learnt from: darklow
Repo: superset-sh/superset PR: 3193
File: apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/NewTabDropZone.tsx:54-57
Timestamp: 2026-04-06T00:08:45.758Z
Learning: In `apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/NewTabDropZone.tsx`, the unconditional `pl-2` on the container is intentional even in vertical mode — it prevents the [+] add button from touching the left border, which would look bad. Do not flag this as a misalignment or suggest removing/conditionalizing `pl-2`.
Learnt from: rajinsyed
Repo: superset-sh/superset PR: 2722
File: apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx:50-55
Timestamp: 2026-03-22T05:00:07.808Z
Learning: In `apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx`, `workspaceTabs` keeps **all** workspace `TabView` wrappers mounted (toggling `display: none` for inactive tabs) to prevent Electron `<webview>` reparenting and page reloads. However, `TabPane` conditionally renders `<Terminal>` only when the tab is active (`activeTabIds[workspaceId] === tabId`), so terminal stream subscriptions and xterm instances are still freed on tab switch. The net result is a hybrid keepalive: browser/webview panes survive tab switches; terminal panes unmount and remount as before. Do not flag background webview keepalive as a bug, and do not flag terminal conditional rendering as incorrect.
Learnt from: AviPeltz
Repo: superset-sh/superset PR: 2889
File: apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts:309-316
Timestamp: 2026-03-26T15:39:01.655Z
Learning: In superset-sh/superset (`apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts`), when an external worktree is imported into Superset (via `createWorkspaceFromExternalWorktree`, which sets `createdBySuperset: false`), deleting the workspace via Superset is intentionally expected to also delete the worktree from disk. Do not flag `removeWorktreeFromDisk()` being called unconditionally (ignoring the `createdBySuperset` flag) as a bug — this is the intended ownership model: import = Superset takes ownership, delete = full disk removal.
Learnt from: kirankunigiri
Repo: superset-sh/superset PR: 2511
File: apps/desktop/src/renderer/stores/tabs/store.ts:2092-2100
Timestamp: 2026-03-16T08:41:29.886Z
Learning: In superset-sh/superset (apps/desktop), the Zustand store merge function in `apps/desktop/src/renderer/stores/tabs/store.ts` intentionally coerces `pane.workspaceRun.state === "running"` to `"stopped-by-exit"` during hydration. This is by design: on a full app restart the daemon is dead, so pessimistic hydration is correct. On renderer-only refresh, `useTerminalLifecycle` remounts within milliseconds and calls `terminal.getSession` to correct the state if the session survived. The brief "stopped-by-exit" flash is acceptable and preferable to stale green badges that could cause incorrect toggle behavior (e.g. attempting to stop a dead process).
Learnt from: zmanian
Repo: superset-sh/superset PR: 2211
File: apps/desktop/src/renderer/screens/main/components/StartView/index.tsx:40-44
Timestamp: 2026-03-08T18:46:30.307Z
Learning: In `apps/desktop/src/renderer/screens/main/components/StartView/index.tsx`, `openNew()` can return multiple projects (multi-select is supported by the backend), but only the first project's id is passed to `openNewWorkspaceModal()`. This is intentional: the workspace modal is designed for a single project at a time, and opening multiple modals in sequence was considered a worse UX. Do not flag this as a bug.
Author
|
I will get back to this PR shortly. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/plans/20260408-1200-workspace-number-badges.md`:
- Around line 25-34: Update the living plan markdown to reflect the implemented
feature: mark completed checklist items (e.g., Modifier key state store,
Modifier extraction hook, local-db tRPC procedures, WorkspaceShortcutBadge,
WorkspaceListItem badge rendering, WorkspaceSidebar mount listener, Collapsed
project header badges, Settings UI toggle, Final validation) as checked and
remove any references to the removed prop WorkspaceDiffStats.hidden; ensure
other mentions noted in the comment (around the three referenced blocks) are
updated or deleted so the plan matches the final implementation and PR summary.
🪄 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
Run ID: 370c003c-56fb-49bc-86ab-783bdfc81d4a
📒 Files selected for processing (3)
apps/desktop/plans/20260408-1200-workspace-number-badges.mdapps/desktop/plans/pr-workspace-number-badges.mdapps/desktop/src/lib/trpc/routers/settings/index.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/plans/pr-workspace-number-badges.md
Comment on lines
+25
to
+34
| - [ ] Modifier key state store (Zustand) | ||
| - [ ] Modifier extraction hook + listener hook | ||
| - [ ] Setting in local-db schema + tRPC get/set procedures | ||
| - [ ] WorkspaceShortcutBadge component | ||
| - [ ] WorkspaceDiffStats hidden prop | ||
| - [ ] WorkspaceListItem badge rendering + enlarged hover hint | ||
| - [ ] Mount listener in WorkspaceSidebar | ||
| - [ ] Collapsed project header badges | ||
| - [ ] Settings UI toggle | ||
| - [ ] Final validation (typecheck, lint, manual test) |
Contributor
There was a problem hiding this comment.
Sync the living plan with the final implementation.
The progress checklist is still fully unchecked, and the plan still documents adding/using WorkspaceDiffStats.hidden, but the PR summary says the feature is implemented and that prop was removed. Please update or remove the stale plan details before merging.
Also applies to: 130-132, 233-234, 274-280
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/plans/20260408-1200-workspace-number-badges.md` around lines 25
- 34, Update the living plan markdown to reflect the implemented feature: mark
completed checklist items (e.g., Modifier key state store, Modifier extraction
hook, local-db tRPC procedures, WorkspaceShortcutBadge, WorkspaceListItem badge
rendering, WorkspaceSidebar mount listener, Collapsed project header badges,
Settings UI toggle, Final validation) as checked and remove any references to
the removed prop WorkspaceDiffStats.hidden; ensure other mentions noted in the
comment (around the three referenced blocks) are updated or deleted so the plan
matches the final implementation and PR summary.
tvrmsmith
force-pushed
the
feat/workspace-number-badges
branch
2 times, most recently
from
40dd818 to
899ee15
Compare
tvrmsmith
force-pushed
the
feat/workspace-number-badges
branch
2 times, most recently
from
3033ff4 to
cc5081f
Compare
Badge labels now derive from the actual hotkey binding via formatHotkeyDisplay, so they reflect platform (⌘1 on Mac, Ctrl+Shift+1 on Windows/Linux) and user overrides. Also moves the modifier-held badge outside the diff stats grid to prevent inheriting diff stats sizing.
Remove dead `hidden` prop from WorkspaceDiffStats, fix inaccurate JSDoc on WorkspaceShortcutBadge, and add missing local-db migration for the showWorkspaceNumbersOnModifier settings column.
…ehavior The captureHotkeyFromEvent implementation intentionally accepts bare alt+key on Mac (Option is a legitimate shortcut modifier). The test was written before this behavior was added and expected null.
Upstream changed ShortcutBinding from a bare chord string to
`string | { version, mode, chord }`. Extract `.chord` before passing to
the local parseBinding helper / formatHotkeyDisplay. Update the
captureHotkeyFromEvent test to read `.codeChord` from the new return shape.
In production builds the env var must not be baked in — a stray value inherited from a dev shell would silently redirect ~/.superset to ~/.superset-<value>, hiding all user data. Only apply in dev where each worktree's setup.sh writes a per-workspace value to .env.
tvrmsmith
force-pushed
the
feat/workspace-number-badges
branch
from
cc5081f to
7be8184
Compare
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
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
2 participants
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.
Links
apps/desktop/plans/20260408-1200-workspace-number-badges.mdSummary
Why / Context
Users don't know they can press ⌘1–⌘9 to jump between workspaces. This feature surfaces those shortcuts on modifier hold, similar to how Arc browser reveals tab numbers.
How It Works
useWorkspaceShortcutModifiershook parses JUMP_TO_WORKSPACE_1–9 bindings to extract modifier keys and generate platform-specific display labels viaformatHotkeyDisplayuseModifierKeyListenerhook attaches keydown/keyup/blur listeners, computesisModifierHeldby checking held keys against the shortcut combosWorkspaceShortcutBadgepresentational pill component used for both modifier-held and hover statesshowWorkspaceNumbersOnModifiercolumn, exposed through tRPC get/set proceduresManual QA Checklist
Modifier Hold Behavior
Hover Behavior
Settings
Collapsed Project Headers
Edge Cases
Testing
bun test— 9 tests pass (modifier key store + shortcut modifier parsing)npx biome check— cleanNew Files
stores/modifier-key-state.ts+.test.ts— Zustand store for held modifier keyshooks/useWorkspaceShortcutModifiers.ts+.test.ts— Parses shortcut bindingshooks/useModifierKeyListener.ts— Side-effect hook for keydown/keyupWorkspaceShortcutBadge.tsx— Presentational badge componentSummary by cubic
Show numbered workspace shortcut badges when holding the workspace modifier key to make jump shortcuts easy to discover. Adds an opt‑in setting; badges appear on workspace rows, on hover next to the close button, and on collapsed project headers.
New Features
formatHotkeyDisplay; respects platform and user overrides; collapsed project headers show aggregated badges.Bug Fixes
ShortcutBindingobject form by extractingchordbefore parsing/formatting.WorkspaceShortcutBadgedocs; add the missing local‑db migration for the new setting.process.env.SUPERSET_WORKSPACE_NAMEin production builds to avoid redirecting user data; only define it in dev.Written for commit 7be8184. Summary will update on new commits.
Summary by CodeRabbit