feat(studio): split the workspace centre into columns of tabs - #409
Conversation
Re-applies the intent of #396 on current main. That branch is 51 commits behind and conflicts in eight files; a third of its diff embeds the Distribution app, whose component tree main has since deleted in favour of Growth. This is the same feature written against the code as it stands. The centre of the workspace is a tab strip. Every open app stays mounted, so switching keeps drafts, scroll position and undo history. Open, close, middle-click close, drag to reorder, Alt+Arrow to reorder, Delete or Backspace to close, overflow scroll, and an empty-workspace state. Claude Artifacts and Coding sessions now open inside the workspace rather than navigating away; both keep their standalone routes, which pass no callbacks and behave exactly as before. Growth keeps `external: true` — a tab cannot hold a route tree with its own layout and rail. The Studio drawer becomes a picker: it no longer hosts panes, so the Expand button has nothing left to do. Its right-click menu survives the rewrite. Two things #396 needed that main solved on its own road, and are not here: the Settings navigation key (main's rebuilt hub already re-targets on a hash change) and the deletion of `external` (Growth needs it). Differences from #396 worth review: - A reorder names the neighbour it lands in front of, not an index. The strip renders a permission-filtered list while the reducer holds the unfiltered one, so an index addressed the wrong slot whenever the two disagreed. - The mindmap's "am I active" test is a prop threaded from the tab host, not a DOM probe. #396 keyed off `[role="tablist"]`, which the source rail also has, so the editor would have gone deaf whenever focus sat in the sidebar. The branch presenter, added to main after #396, is gated the same way. - `?feature=workflows`, `?feature=analytics` and `?feature=metadata` open their panes again. A tab needs a label and an icon, so every id the shell can open now has to resolve to a feature; naming these three is what that costs, and it retires three "Unknown feature" screens. - The shell's window shortcut listener skips keys a focused control already consumed, so Delete on a tab no longer also runs a bound command. - 26 unit tests for the strip, which #396 shipped without, plus a /dev/studio-tabs preview route for looking at it without signing in. Verified: workspace typecheck, repository lint (no new warnings), 3,043 web tests. Exercised in a browser — six panes mounted at once with one visible, a draft surviving a switch away and back, keyboard reorder and its announcement, Delete claiming the key, the empty state, and light and dark. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_1aa87a1f-650c-4934-b1d2-43084716dc62) |
The chat panel's avatar menu navigates to /employer/settings. That was fine when one pane was mounted; with tabs it leaves the workspace and closes every open app to reach something that is one tab away. Settings opens in its tab instead. The hash still carries the section, which is how the settings panel picks one whether it is already mounted or not. Anything pointing elsewhere navigates as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b4876162-62b4-47ea-aee0-54d11720b051) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb70dd36b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <MindmapEditorHost | ||
| key={editedMindmap.mindmapId} | ||
| mindmapId={editedMindmap.mindmapId} | ||
| active={paneActive && !viewerSource} |
There was a problem hiding this comment.
When a map is opened through ?source=<id>&edit=1, viewerSource remains set to that map while the Mindmap tab is displayed, so this expression always passes active={false}. The newly added activity gates then disable the editor's keyboard shortcuts, clipboard paste, and Space-to-pan immediately after entering Edit, while workspace-level shortcuts remain enabled. The active check needs to distinguish a preview overlay (viewerSource && !editing) from the source that is currently being edited.
Useful? React with 👍 / 👎.
| // Gated rather than unmounted: unmounting runs the cleanup that | ||
| // unfolds the map, so hiding the tab would lose the presentation. | ||
| if (isActive && !isActive()) return; | ||
| if (e.defaultPrevented) return; |
There was a problem hiding this comment.
In focus-mode presentation, the first go() changes step, causing this Presenter effect to remove and re-add its listener behind MindmapEditor's persistent presenting listener. On the next Arrow/Page/Space key, the parent listener now calls preventDefault() first, so this new guard returns before go() and branch-by-branch presentation becomes stuck after one step. The page-level handler must skip focus-mode presentation, or the two handlers need another way to identify which one owns the event.
Useful? React with 👍 / 👎.
| } | ||
| // The host's library is what Open and Continue read from, so it | ||
| // has to see the new document before either becomes useful. | ||
| if (stored.length > 0) void onImported?.(); |
There was a problem hiding this comment.
After an embedded session import, setPreview immediately exposes the Open button while this host refresh is fire-and-forget. useWorkspaceData.refresh() does not set sourcesLoading back to true, so clicking Open before that request finishes makes the shell treat the new document as nonexistent and remove its source parameter. Await the host refresh before marking the row usable, or represent the imported source/loading state optimistically.
Useful? React with 👍 / 👎.
|
CI is green on
This supersedes #396, which cannot be rebased: roughly a third of its diff embeds the Distribution app, whose component tree main deleted in favour of Growth. Suggest closing #396 in favour of this one rather than merging it. |
The centre is one or more columns side by side, each its own strip of tabs. Chat beside a document, or chat beside a tool beside a document, up to three with a draggable divider. Ways in: "Open to the side" on any source, in the rail and in Knowledge; "Split to the right" on a tab's menu and on the strip; dragging a tab from one column into another; and four new bindings in Settings → Shortcuts (split, focus the next and previous column, close the current app). Split and focus are app actions, so they are in the command palette too. The strip is also restyled. It was a VS Code transplant — a 2px brand rule over the active tab, hard dividers between tabs, a permanent close box. The rest of this workspace draws "this one is current" as a raised pill in a recessed track, which is what the source rail's own Sources/History control does, so the strip does that now. The part worth reviewing is how panes are hosted. They are NOT rendered inside their column. Each gets a host element StudioSplitView creates once and then moves with appendChild into whichever column shows it. Portalling into the column's own slot is the obvious approach and is wrong: React compares a portal's containerInfo when it reconciles, so changing it unmounts the subtree and mounts a new one — losing exactly the draft, scroll and undo that tabs exist to keep. StudioSplitView.test.tsx asserts the DOM node is identical across a move and that a mount counter stays at 1, because that regression is invisible on any pane without state. Three other things that bite: - Slots register from a layout effect in a small PaneSlot child, not an inline ref callback. An inline callback is a new function each render, so React calls it with null and then the element every pass, which never settles when the host keeps slots in state. - Visible is no longer the same as focused. One pane per column is visible but only one is focused, so the mindmap editor now gates its window listeners on its column having the focus. Gating on visibility alone killed every workspace shortcut while typing in the chat beside an open map. - The palette, Studio and avatar controls are drawn by the leftmost column only. They belong to the workspace, and three columns meant three avatars. Also: the document viewer gained an in-flow mode, and folds its versions and notes rail into a Details panel below 760px so a narrow column shows the document rather than a sliver of it. 42 new tests (23 layout, 12 strip, 7 split view). Verified in a browser against the real workspace: a document opened beside the chat, a third column added, a tab dragged between columns collapsing the one it left, one Studio control across three columns, and a PDF filling its column. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Five reviewers over the new columns, each finding verified by a second agent that tried to refute it. Thirty-seven survived, deduplicating to these. Two were serious: - A source tab could not be re-selected by clicking it. Selection was routed through `expandFeature`, which resolves Studio features and silently ignores anything else, so a column holding two documents could only ever show the second. Selection is now its own verb. - The workspace lost its palette, Studio menu and avatar entirely whenever the leftmost column held a document, and showed three of each across three columns. They were a property of whichever pane happened to be leftmost. They are workspace chrome, so they now live in the leftmost strip and are drawn once regardless of what is in it. The rest, by theme. Keyboard: the column verbs escaped the mindmap's keyboard gate, which was otherwise the one thing stopping someone leaving a focused map. Two of the four new defaults were unreachable — ⌘⇧W closes the browser window — and all four were "outside-input", so none worked from the composer. They are now ⌘⌥\, ⌘⌥[, ⌘⌥] and ⌘⌥W, and they always fire. Focus: only a pointer claimed a column, so tabbing into one left the verbs acting on another; focus now follows either. Closing a column's last tab stranded focus on the body, because the strip that tries to recover it dies with the column — the recovery moved up to the split view, which survives. Panes: `appendChild` takes a node out of the document, which blurs what was focused in it and resets every scroller inside. It now prefers `moveBefore`, guarded by `isConnected` because it throws on a detached node, and otherwise snapshots focus and scroll and puts them back. Splitting also rebuilt every column to its right, because each was an array rather than a keyed child. And one render in which a source could not be named destroyed its pane; hosts now follow the reducer, and only the strip filters. Documents: an embedded viewer's Escape listener is on the window, so it closed every open document tab from anywhere — it is for the overlay only now. Its ResizeObserver read width 0 for a hidden pane and collapsed the rail on every background tab. Opening or citing a source already open in a column now brings that column forward instead of laying an overlay over the split. Appearance: the active pill was about 1.05:1 against its own track, and nothing said which column had the focus. Both are fixed by using what the rest of the workspace already uses for the current row — brand-soft over brand ink — with a quiet ring for the unfocused column. Also: the close × leaves the tablist's owned children, dark-mode hover is neutral rather than brand, the drop indicator is no longer clipped by the scroll container, and the empty state renders inside the strip so an empty workspace keeps its chrome. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_5e14d041-bea5-4759-8aa0-fff06e8d0a99) |
Deodat-Lawson
changed the title
…est-daf4af # Conflicts: # apps/web/src/app/employer/documents/_workspace/StudioDrawer.tsx # apps/web/src/app/employer/documents/_workspace/WorkspaceShell.tsx
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_8b8eb89d-df61-4bab-9187-e6b56fa1d1ee) |
|
CI is green on
Two notes on the history here:
|
1 participant
Summary
The workspace centre becomes one or more columns side by side, each its own strip of tabs. Chat beside a document, or chat beside a tool beside a document, up to three with a draggable divider. Every open pane stays mounted, so switching tabs — and moving a tab between columns — keeps drafts, scroll and undo.
This supersedes #396, which cannot be rebased: about a third of its diff embeds the Distribution app, whose component tree main deleted in favour of Growth. Suggest closing it in favour of this.
What you can do
The part worth reviewing
Panes are not rendered inside their column. Each gets a host element
StudioSplitViewcreates once and then moves into whichever column shows it. Portalling straight into the column's slot is the obvious approach and is wrong: React compares a portal'scontainerInfowhen it reconciles, so changing it unmounts the subtree and mounts a new one, losing exactly the state tabs exist to keep.StudioSplitView.test.tsxasserts the DOM node is identical across a move and a mount counter stays at 1, because that regression is invisible on any pane without state.The move prefers
moveBeforeso the browser does not blur the caret or reset scrollers, guarded byisConnectedbecause it throws on a detached node, with a snapshot-and-restore fallback everywhere else.Also here
?feature=workflows,?feature=analyticsand?feature=metadataopen their panes again instead of showing "Unknown feature".Review
Five reviewers over the diff, each finding verified by a second agent that tried to refute it; 37 survived and are fixed in
e2983d5c, whose message lists them. The two that mattered: a source tab could not be re-selected by clicking it, and the workspace lost its chrome entirely when the leftmost column held a document.Testing
pnpm -r typecheck— passes in every workspace.pnpm lint— 0 errors; 63 warnings, the same 63 as on main, none in a file this branch touches. NeedsNODE_OPTIONS=--max-old-space-size=8192locally or ESLint exhausts the heap and prints a partial report that looks clean.pnpm --filter @launchstack/web test— 275 suites, 3,059 tests, 0 failures. 42 new (23 layout, 12 strip, 7 split view).Not exercised: dragging a tab to the far edge to create a column (only dragging between existing ones is built), and persisting the layout across reloads.
🤖 Generated with Claude Code
Note
Medium Risk
Large refactor of the documents workspace shell (layout, focus, DOM reparenting, and keyboard routing); high user-facing impact but localized to employer documents UI with substantial new test coverage.
Overview
The workspace centre is now up to three resizable columns, each with its own tab strip. Users can open sources to the side, split to the right, and drag tabs between columns; open panes stay mounted so drafts, scroll, and undo survive tab switches and column moves.
StudioSplitViewhosts panes in stable DOM hosts that are reparented between column slots (withmoveBeforewhen possible), avoiding portal container changes that would remount React subtrees.paneLayoutowns layout state;StudioTabshandles strip UX (drag/drop, keyboard reorder, focus). Palette, Studio, and avatar chrome move to the leftmost strip only;AskPanelno longer renders those controls.Studio becomes a dialog picker (no inline panes or Expand). Artifacts and agent sessions mount as Studio tabs with optional host callbacks so gallery/viewer and Open/Continue stay in-workspace; standalone routes unchanged.
resolveStudioFeature/demotedFeatureHreffix deep links for workflows, analytics, and metadata.Mindmap editors gate window keyboard, paste, and canvas on active/focused pane so background tabs do not steal input.
DocumentVieweraddsembeddedin-column layout and collapses the versions/notes rail into a Details overlay when the column is narrow. Docs, changelog, registry tests, and a dev studio-tabs preview accompany the change.Reviewed by Cursor Bugbot for commit f4f187a. Bugbot is set up for automated code reviews on this repo. Configure here.