Share draggable workspace pane interactions#443
Merged
Conversation
roborev: Combined Review (
|
roborev: Combined Review (
|
mariusvniekerk
force-pushed
the
refactor-draggable-pane-component
branch
from
3d2dd64 to
5f84098
Compare
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
Collaborator
Author
|
going to handle that regression as a follow up instead |
The workflow panes had tab dragging, tab sorting, split-drop previews, and divider resizing embedded in one terminal-specific component. The same interaction model is expected for PR/activity panes, so keeping it local would force the next panel surface to copy a fragile set of drag-state rules. This keeps SplitResizeHandle available for simpler dedicated sidebars while moving only the tabbed workspace behavior into a shared UI primitive that accepts arbitrary panel content and workflow-specific drag bridging. Validation:\n- bun run --cwd frontend typecheck\n- bun run --cwd packages/ui typecheck\n- bun run --cwd frontend test -- ../packages/ui/src/components/shared/TabbedPanelTree.test.ts src/lib/components/terminal/WorkspaceTerminalView.test.ts\n- bun run --cwd frontend lint\n- bun run --cwd packages/ui lint\n- bun run --cwd frontend test:e2e:mock -- --project=chromium tests/e2e/workspace-sidebar.spec.ts Generated with Codex
Roborev caught that action buttons supplied through snippets were outside TabbedPanelTree scoped CSS, which meant workflow tab controls could lose their button chrome after the extraction. The shared component now styles the neutral action hook across the snippet boundary. The shared panel surface also should not carry workflow compatibility aliases into future PR/activity panes, so this removes the old workflow/group selectors, updates tests to the neutral tabbed-panel contract, and documents callback/read-only and accessibility scope. Validation:\n- bunx @sveltejs/mcp@0.1.22 svelte-autofixer packages/ui/src/components/shared/TabbedPanelTree.svelte --svelte-version 5\n- bun run --cwd frontend typecheck\n- bun run --cwd packages/ui typecheck\n- bun run --cwd frontend test -- ../packages/ui/src/components/shared/TabbedPanelTree.test.ts src/lib/components/terminal/WorkspaceTerminalView.test.ts\n- bun run --cwd frontend lint\n- bun run --cwd packages/ui lint\n- bun run --cwd frontend test:e2e:mock -- --project=chromium tests/e2e/workspace-sidebar.spec.ts\n- bun run --cwd frontend test:e2e -- --project=chromium tests/e2e-full/workspace-tab-persistence.spec.ts Generated with Codex
The shared tabbed panel workspace now has a neutral demo surface so future PR and activity pane work can validate drag, split, tab, and resize behavior without reaching through the terminal workflow. The repo picker also needed visible typeahead dropdown examples. The mock repo fixture now matches the required repo response shape so the design-system e2e exercises the same data contract the picker consumes. Validation: - bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/design-system/DesignSystemPage.svelte --svelte-version 5 - bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/design-system/DesignSystemTabbedPanelDemo.svelte --svelte-version 5 - bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/design-system/DesignSystemTypeaheadDemo.svelte --svelte-version 5 - bun scripts/check-font-size-tokens.ts - bun run --cwd frontend typecheck - bun run --cwd frontend lint - node ./node_modules/@playwright/test/cli.js test --config=playwright.config.ts --project=chromium tests/e2e/design-system.spec.ts --workers=1 (from frontend/) - git diff --cached --check Generated with Codex
Dragging a tab into another split leaf only marked the destination tab as a sort target because drag preview state lived in the source leaf instance. That made the Terminal-to-Overview path on the design-system page feel like it was not a valid drop, even though the shared drag payload could be read across leaves. The tabbed panel splitter also used an inset pseudo-line inside a gutter, which looked heavier and messier than the regular resize handle treatment. The divider is now a solid no-padding 3px handle using the shared border/accent colors. Validation: - bunx @sveltejs/mcp@0.1.22 svelte-autofixer packages/ui/src/components/shared/TabbedPanelTree.svelte --svelte-version 5 - node ./node_modules/vitest/vitest.mjs run ../packages/ui/src/components/shared/TabbedPanelTree.test.ts (from frontend/) - node ./node_modules/@playwright/test/cli.js test --config=playwright.config.ts --project=chromium tests/e2e/design-system.spec.ts --workers=1 (from frontend/) - bun run --cwd packages/ui typecheck - bun run --cwd packages/ui lint - bun run --cwd frontend typecheck - bun run --cwd frontend lint - git diff --check - In-app browser computed style: design-system splitter width 3px, no padding, no side borders Generated with Codex
The design-system tabbed panel still read wider than its divider because pane borders contributed visual thickness around the handle. Collapsing the split-adjacent borders makes the measured and perceived separator match. The demo also had a placeholder tab action marker and a top border above the tab strip, both of which made the example look like debug scaffolding instead of a reusable panel primitive. Validation: - bunx @sveltejs/mcp@0.1.22 svelte-autofixer packages/ui/src/components/shared/TabbedPanelTree.svelte --svelte-version 5 - bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/design-system/DesignSystemTabbedPanelDemo.svelte --svelte-version 5 - node ./node_modules/vitest/vitest.mjs run ../packages/ui/src/components/shared/TabbedPanelTree.test.ts (from frontend/) - PLAYWRIGHT_PORT=64724 PLAYWRIGHT_REUSE_EXISTING_SERVER=1 node ./node_modules/@playwright/test/cli.js test --config=playwright.config.ts --project=chromium tests/e2e/design-system.spec.ts --workers=1 (from frontend/) - bun run --cwd packages/ui typecheck - bun run --cwd packages/ui lint - bun run --cwd frontend typecheck - bun run --cwd frontend lint - git diff --check - In-app browser computed style: 0 action buttons, effective separator 3px, no split-adjacent or top leaf borders Generated with Codex
Remove the stale workflow stage inset now that tabbed panel chrome owns its own border treatment. Add an e2e layout assertion so the workflow panel stays edge-to-edge inside the terminal workspace. Validation: bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/terminal/WorkspaceTerminalView.svelte --svelte-version 5; node ./node_modules/@playwright/test/cli.js test --config=playwright.config.ts --project=chromium tests/e2e/workspace-sidebar.spec.ts -g 'renders workflow panes flush with the workspace stage' --workers=1; bun run --cwd frontend typecheck; bun run --cwd frontend lint; git diff --check
Represent terminal workspace routes with the single canonical Workspaces nav option so the compact page menu no longer duplicates Workspaces. Add the missing left frame line on the terminal title and workflow rows so the panel chrome lines up with the tabbed panes. Validation: bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/layout/AppHeader.svelte --svelte-version 5; bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/terminal/WorkspaceTerminalView.svelte --svelte-version 5; node ./node_modules/vitest/vitest.mjs run src/lib/components/layout/AppHeader.test.ts; node ./node_modules/@playwright/test/cli.js test --config=playwright.config.ts --project=chromium tests/e2e/workspace-sidebar.spec.ts -g 'renders workflow panes flush|shows one Workspaces option' --workers=1; bun run --cwd frontend typecheck; bun run --cwd frontend lint; git diff --check
Allow active TabbedPanelTree panels to scroll vertically when arbitrary panel content exceeds the panel body. Expand the design-system tabbed panel demo with overflowing content and assert the panel wrapper is scrollable. Validation: bunx @sveltejs/mcp@0.1.22 svelte-autofixer packages/ui/src/components/shared/TabbedPanelTree.svelte --svelte-version 5; bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/design-system/DesignSystemTabbedPanelDemo.svelte --svelte-version 5; node ./node_modules/@playwright/test/cli.js test --config=playwright.config.ts --project=chromium tests/e2e/design-system.spec.ts -g 'design system page renders panel and typeahead examples' --workers=1; bun run --cwd frontend typecheck; bun run --cwd frontend lint; bun run --cwd packages/ui typecheck; bun run --cwd packages/ui lint; node ./node_modules/vitest/vitest.mjs run ../packages/ui/src/components/shared/TabbedPanelTree.test.ts; git diff --check
Selected tabs should merge into the tab strip by covering the shared rule, not by drawing their own bottom border. This keeps the reusable tabbed panel chrome crisp in both the design-system demo and workflow panes. Validation: bunx @sveltejs/mcp@0.1.22 svelte-autofixer packages/ui/src/components/shared/TabbedPanelTree.svelte --svelte-version 5; in-app browser computed style for active overview tab reports borderBottomWidth=0px and marginBottom=-1px; node ./node_modules/@playwright/test/cli.js test --config=playwright.config.ts --project=chromium tests/e2e/design-system.spec.ts -g 'design system page renders panel and typeahead examples' --workers=1; bun run --cwd packages/ui typecheck; bun run --cwd packages/ui lint; bun run --cwd frontend typecheck; bun run --cwd frontend lint; node ./node_modules/vitest/vitest.mjs run ../packages/ui/src/components/shared/TabbedPanelTree.test.ts; git diff --check; git diff --cached --check Generated with Codex
Tabbed panel tabs should sit above the tab strip divider instead of drawing or revealing a selected-tab bottom rule. The shared panel body also should not reserve scrollbar gutter by default, because workflow panes such as Workspace Home own their own scrolling and otherwise appear narrower than the surrounding panel chrome. The design-system demo opts into scrollable tab panels so overflow examples remain visible without forcing that gutter onto workflow panes. Validation: bunx @sveltejs/mcp@0.1.22 svelte-autofixer packages/ui/src/components/shared/TabbedPanelTree.svelte --svelte-version 5; bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/design-system/DesignSystemTabbedPanelDemo.svelte --svelte-version 5; in-app browser workflow metrics show handleWidth=4, homeToPanelRight=0, panelOverflowY=hidden; node ./node_modules/@playwright/test/cli.js test --config=playwright.config.ts --project=chromium tests/e2e/design-system.spec.ts -g 'design system page renders panel and typeahead examples' --workers=1; node ./node_modules/@playwright/test/cli.js test --config=playwright.config.ts --project=chromium tests/e2e/workspace-sidebar.spec.ts -g 'clicking PR segment opens sidebar with PR content' --workers=1; bun run --cwd packages/ui typecheck; bun run --cwd packages/ui lint; bun run --cwd frontend typecheck; bun run --cwd frontend lint; node ./node_modules/vitest/vitest.mjs run ../packages/ui/src/components/shared/TabbedPanelTree.test.ts; git diff --check; git diff --cached --check Generated with Codex
Docked terminal panes should share the same flush split treatment as the workflow panel instead of showing inset gutters, duplicate borders, or xterm black viewport gaps near pane dividers. Those artifacts made split terminals read as padded panels rather than a single draggable workspace surface. The bottom dock resize affordance keeps its larger hit target while presenting the same divider weight as the surrounding panel chrome. Nested terminal splits now collapse adjacent leaf borders and use a solid divider surface so the visual separator is exactly the separator. Validation: bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/terminal/TerminalSplitTree.svelte --svelte-version 5; bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/terminal/DockedTerminalPanel.svelte --svelte-version 5; bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/terminal/XtermTerminalPane.svelte --svelte-version 5; in-app browser seam metrics show 3px divider, 0px adjacent leaf borders, and xterm viewport background rgb(13, 17, 23); node ./node_modules/@playwright/test/cli.js test --config=playwright.config.ts --project=chromium tests/e2e/workspace-sidebar.spec.ts -g 'opens the plain shell from the bottom terminal panel|renders bottom terminal split panes flush with consistent dividers' --workers=1; bun run --cwd frontend typecheck; bun run --cwd frontend lint; node ./node_modules/vitest/vitest.mjs run src/lib/components/terminal/WorkspaceTerminalView.test.ts src/lib/components/terminal/TerminalPane.test.ts; git diff --check; git diff --cached --check Generated with Codex
Active terminal panes should use the same top-accent language as tabbed workspace panels without changing box metrics or tinting side borders. Switching between split panes must move only the selected-state paint, not the header position or pane geometry. Panel chrome widths now use named design tokens instead of local pixel literals, so terminal panes and the shared TabbedPanelTree stay aligned when divider or accent weights need tuning. Validation: bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/terminal/TerminalSplitTree.svelte --svelte-version 5; bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/terminal/DockedTerminalPanel.svelte --svelte-version 5; bunx @sveltejs/mcp@0.1.22 svelte-autofixer packages/ui/src/components/shared/TabbedPanelTree.svelte --svelte-version 5; in-app browser tab-switch metrics show stable headerTop/headerHeight and active header box-shadow accent only; node ./node_modules/@playwright/test/cli.js test --config=playwright.config.ts --project=chromium tests/e2e/workspace-sidebar.spec.ts -g 'opens the plain shell from the bottom terminal panel|renders bottom terminal split panes flush with consistent dividers' --workers=1; bun run --cwd frontend typecheck; bun run --cwd frontend lint; bun run --cwd packages/ui typecheck; bun run --cwd packages/ui lint; node ./node_modules/vitest/vitest.mjs run src/lib/components/terminal/WorkspaceTerminalView.test.ts ../packages/ui/src/components/shared/TabbedPanelTree.test.ts; git diff --check; git diff --cached --check Generated with Codex
Roborev caught two risks in the reusable pane work: optional drag callbacks could still expose unavailable sort behavior, and the design-system page was not proving the split, resize, reorder, and dropdown states future PR/activity panes will rely on. The shared tabbed panel now keeps append-only drops on the append path, nested split chrome stays collapsed at parent dividers, and the design-system fixture shows a real open typeahead state without assuming a specific hard-coded repository exists. Validation: bunx @sveltejs/mcp@0.1.22 svelte-autofixer packages/ui/src/components/shared/TabbedPanelTree.svelte --svelte-version 5; bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/RepoTypeahead.svelte --svelte-version 5; bunx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/design-system/DesignSystemTypeaheadDemo.svelte --svelte-version 5; node ./node_modules/vitest/vitest.mjs run ../packages/ui/src/components/shared/TabbedPanelTree.test.ts src/lib/components/RepoTypeahead.test.ts (from frontend/); bun run --cwd packages/ui typecheck; bun run --cwd frontend typecheck; bun run --cwd packages/ui lint; bun run --cwd frontend lint; PLAYWRIGHT_PORT=64724 PLAYWRIGHT_REUSE_EXISTING_SERVER=1 node ./node_modules/@playwright/test/cli.js test --config=playwright.config.ts --project=chromium tests/e2e/design-system.spec.ts --workers=1 (from frontend/); PLAYWRIGHT_PORT=64724 PLAYWRIGHT_REUSE_EXISTING_SERVER=1 node ./node_modules/@playwright/test/cli.js test --config=playwright.config.ts --project=chromium tests/e2e/workspace-sidebar.spec.ts --workers=1 (from frontend/); node ./node_modules/@playwright/test/cli.js test --config=playwright-e2e.config.ts --project=chromium workspace-tab-persistence.spec.ts --workers=1 (from frontend/); git diff --check Generated with Codex
The rebase exposed two local verification blockers before the PR could be pushed: generated Playwright output was being formatted as source, and Pierre annotation remounts could create Svelte components while a previous annotation was still tearing down. Both made otherwise unrelated full-suite verification fail locally, which is exactly the state this branch should not publish. The annotation host remains synchronous for Pierre's slot API, but the Svelte component mount is deferred until the host is connected and cancelled if the host disappears first. Root frontend checks now ignore generated test-results artifacts for the same reason the directory is gitignored. Validation: make test; bun run --cwd frontend test ../packages/ui/src/components/diff/DiffFile.test.ts; bun run --cwd frontend test; make frontend-check; make guardrail-check; make vet; make rust-test; make test-e2e. Blocking: bun run --cwd frontend test:e2e:mock still fails in stale broad mock specs, so this commit is not being pushed yet. Generated with Codex Co-authored-by: Codex <codex@openai.com>
The reusable pane branch was blocked on stale frontend tests that still assumed legacy PR routes, missing stable DOM hooks, and shortcut behavior that varied across browsers. Those gaps made local verification fail even though the pane implementation itself was ready to push. The mocks now model provider-aware PR, issue, repo, and approval routes used by the app, and the keyboard/modal paths cover the actual browser events seen in e2e. The palette and cheatsheet traps explicitly own Tab navigation so WebKit cannot leak focus out of open dialogs. Validation: make test; make frontend-check; bun run --cwd frontend test; bun run --cwd frontend test:e2e:mock; git diff --check Generated with Codex Co-authored-by: Codex <codex@openai.com>
Local e2e runs were able to leave tmux-backed workspaces behind, and the default all-browser Playwright path let browser projects share one backend lifecycle. That made failures harder to diagnose and allowed state from one project to leak into another instead of exercising isolated test environments. The e2e server now tears down workspaces through the same manager used by the product, while the default wrapper runs Chromium, Firefox, and WebKit as separate invocations. Roborev e2e temp files also live under the repo tmp directory so Docker/Colima can mount the SQLite database reliably. Validation: MIDDLEMAN_E2E_OUTPUT_FILE=/tmp/middleman-e2e-default-isolated-final.log bun run --cwd frontend test:e2e; go test ./cmd/e2e-server -run TestNonExistent -shuffle=on; make frontend-check; bash -n scripts/run-roborev-e2e.sh; git diff --check; git diff --cached --check. Generated with Codex Co-authored-by: Codex <codex@openai.com>
The e2e wrapper needs to keep browser projects isolated even when a developer passes normal Playwright options such as --headed or a spec path. Treating any user arg as a complete override recreated the shared-backend path this wrapper was meant to avoid. Only explicit --project args now opt out of the default per-browser fanout. The runner planning logic is covered by script regression tests, and the pre-commit script-test hook now watches frontend runner scripts so this behavior is checked when the wrapper changes. Validation: bun test scripts/run-e2e-to-file.test.ts; make script-tests; MIDDLEMAN_E2E_OUTPUT_FILE=/tmp/middleman-e2e-arg-isolation.log bun run --cwd frontend test:e2e -- tests/e2e-full/workspace-tab-persistence.spec.ts; make frontend-check; git diff --check; MIDDLEMAN_E2E_OUTPUT_FILE=/tmp/middleman-e2e-default-isolated-review-fix.log bun run --cwd frontend test:e2e (chromium and firefox passed, webkit hit a transient diff-view click failure); MIDDLEMAN_E2E_OUTPUT_FILE=/tmp/middleman-e2e-webkit-context-expansion-rerun.log bun run --cwd frontend test:e2e -- --project=webkit tests/e2e-full/diff-view.spec.ts -g 'context expansion fetches both sides'; MIDDLEMAN_E2E_OUTPUT_FILE=/tmp/middleman-e2e-webkit-full-review-fix-rerun.log bun run --cwd frontend test:e2e -- --project=webkit. Generated with Codex Co-authored-by: Codex <codex@openai.com>
The Chromium e2e failure was checking whether earlier Pierre rows were viewport-visible after the test intentionally scrolled into a later expanded file. The user-visible contract is that those earlier rows remain rendered and nonblank, so the assertion now matches the virtualization behavior it is meant to protect.
mariusvniekerk
force-pushed
the
refactor-draggable-pane-component
branch
from
7971ad0 to
2ec3590
Compare
roborev: Combined Review (
|
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.
PR and activity surfaces are starting to need the same VS Code-like pane behavior that was previously trapped inside the terminal workflow. Keeping drag, tab, split, and resize logic terminal-specific would make every new panel repeat fragile interaction code and drift visually.
This PR creates a reusable tabbed workspace panel model for arbitrary pane content, keeps the dedicated SplitResizeHandle available for simpler sidebar splits, and adds design-system examples so the interaction contract can be validated outside the terminal route.
It also tightens the shared chrome around those panes because the reusable primitive only works well if active tabs, split dividers, scroll gutters, and terminal-docked panels read as one stable workspace surface instead of shifting or accumulating accidental borders.