You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(web): polish preview workspace and unify tab/header controls - #289
Support intentional empty preview groups with predictable target placement, persistence, explicit empty-group closing, and group-scoped tab batch closing.
Move preview title editing into tabs, retain existing save and rename lifecycles, and show external Chat capability-cache status in tab tooltips with visible error recovery.
Refine preview-related Canvas and desktop chrome and simplify Chat controls.
Unify Chat context source chips and clarify included-source, adjacent-node, and Canvas-selection actions.
Restore keyboard focus after closing tabs, tab batches, or empty groups, while preserving retained control focus and the host's final-panel collapse handoff.
Update architecture documentation and regression coverage.
Review
Reviewed the complete original feature branch, then based this PR on the latest origin/main. Earlier Layers, grouped dragging, Canvas chrome, and empty-chat disclaimer changes are already merged through #287 and are not repeated in this PR. The original branch is preserved.
Review covered preview lifecycle transitions, batch-close scope and settlement, title-edit focus handling, empty-group persistence, and common UI consistency. Corrected stale test assertions for the expanded compact Canvas header's 44px height and the shared menu's compact padding. No remaining blocking correctness findings were identified.
Follow-up verification of Copilot's review confirmed and reproduced the two close-action focus findings in a browser; both are now fixed with unit and browser regressions. The missing SelectedNodeRefs import in the originally published snapshot is also repaired by the approved Chat context follow-up. The cross-group activation finding is a false positive for the current UI path: the drag handler calls activateTab immediately after moveTab, and Open to Side also activates the moved tab.
Validation
Validation used an isolated snapshot containing the approved Chat context commit and review fixes, excluding ongoing local node-mention work.
Repository pnpm typecheck: passed across all workspace packages.
Repository pnpm format: passed; no further tracked formatting changes.
Web unit suite: 285 test files, 3,311 tests validated. The initial run passed 3,273 tests; one suite was blocked by the isolated runner's filesystem restriction on symlinked installed PDF worker assets, then passed all 38 tests when that dependency directory was explicitly allowed.
Focus, empty groups, tab chrome, and Chat context browser regressions on the isolated snapshot: 13 passed.
Earlier Preview, menus, popup sizing, title editing, status, and toolbar browser coverage: 17 passed.
Additional original-branch validation: 35 Layers, fullscreen, Canvas chrome, and icon browser tests validated, including the corrected header geometry test.
Repository pnpm lint:fix: passed on the isolated snapshot with 348 existing warnings and no errors.
Translation parity, changed-test formatting/lint, and git diff --check: passed.
Limitations
Electron packaging and Windows-device behavior were not exercised in this review.
Make pinned/uploaded attachment preview tooltips available from keyboard focus.
Replace repeated single-tab deletion with one linear batch removal, retaining topology, focus and editor-settlement contracts.
Virtualize the node mention picker without limiting results; 2,000-result regressions verify at most 16 mounted rows, keyboard wrapping, scrolling, selection and search.
Update the stale Sketch source-count component name and compose-placeholder assertions.
Include the user-approved PDF selection-layout and desktop hover-outline follow-up (b4f42b5).
Validation used an isolated exact snapshot: repository typecheck, format and lint passed (348 existing warnings); translation parity passed; 3,336 web unit tests were validated across 286 files (one inherited placeholder assertion was corrected and its full 9-test suite rerun). The isolated Vitest runner explicitly allowed the existing symlinked dependency asset directory. Expanded browser validation passed 48 of 51 tests, including all new review/PDF regressions. The three failures also reproduced on the unmodified b739efe baseline: Sketch type-button dragging, edge context-toolbar visibility, and the obsolete single stroke-toolbar selector. These baseline failures were not changed or hidden. Electron packaging and Windows-device behavior were not validated.
Support empty groups, scoped tab batch closing, tab-owned title editing and status hints. Unify compact menu density and preview header actions, align fullscreen controls, and move summaries into header popovers.
Includes regression tests and architecture documentation, rebased onto current main without repeating the changes merged in #287.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Consolidate excerpts, attachments, and Canvas selections above the composer with compact wrapping chips and a single leading action. Keep selected nodes last and share composer focus styling across sources.
Add component and browser coverage and update the preview workspace documentation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Recover DOM focus only when a close action removes its focused control, reuse the repaired active tab or empty group's New Chat control, and preserve the host's final-panel focus handoff. Add unit and browser regressions and document the focus contract.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add an accessible @ node picker that preserves draft mentions and stages thread-owned context references. Include localized placeholder hints, refine chip spacing, and distinguish unadded nodes with transparent dashed outlines.
Cover mention interactions and context layout with unit and browser tests, and document the behavior.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keyboard-activating Split removes the focused Split button as soon as the second group renders, but this path does not enroll the control in the existing focus-recovery effect. Focus therefore falls to body instead of the newly active empty group's New Chat control. Capture the active element before splitting so the existing layout effect can restore focus.
Correct stale rename behavior in architecture documentation
docs/architecture/web-architecture.md:314
This documents the new tab-owned rename flow, but the linked authoritative docs/architecture/preview-workspace.md:76 still says clicking the panel header enables inline rename, while its line 116 describes F2/context-menu editing. Update the stale sentence so the architecture source of truth is internally consistent.
Replace card hover shadows with layout-free Canvas HUD outlines. Keep embedded PDF scrollbar layout stable across selection while preserving input ownership. Add unit and browser regressions and update interaction documentation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Share a linear tab-removal primitive, preserve active-tab and empty-group behavior, and avoid rescanning inactive groups during editor settlement. Make attachment previews keyboard-focusable and virtualize mention rows while preserving the full searchable and keyboard-accessible result set.
Add regression coverage for operation count, batch equivalence, keyboard tooltips and 2,000-result mention navigation. Update architecture documentation and stale compose-placeholder assertions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The new title portal is hosted inside the element with role="tab". ARIA tabs force descendant semantics to presentation, so the portalled text input can be omitted or misrepresented in a screen reader's accessibility tree even though DOM focus reaches it. Keep the editor visually in the tab strip, but mount it as a sibling of the role="tab" element (and preserve the tab's labeling relationship) rather than as its descendant.
Empty adjacent labels become inconsistent attachment names
For an adjacent node whose label is an empty string, the button displays “Untitled” via adjacentNodeLabel, but this branch stores label: ''. The resulting attachment chip then falls back to “File,” so the source changes names after it is added. Store the already-normalized display label.
This padding contradicts the new architecture contract at docs/architecture/web-architecture.md:70: px-3 leaves 12px on the right, while the documented compact layout requires 8px right padding (matching the 8px bottom clearance). Use asymmetric padding here.
Wrong empty-group message in preview and split layouts
Every empty group now shows the workspace-level instruction to double-click a Canvas node. That instruction is impossible in Preview fullscreen, where MainLayout removes the Canvas, and is misleading for an inactive half of a split. Use the existing preview.emptyGroup message for split/fullscreen groups and reserve emptyWorkspace for a sole group with the Canvas available.
Preview fullscreen rail metrics are documented incorrectly
docs/architecture/web-architecture.md:80
The statement that the vertical Preview-fullscreen rail remains unchanged is now false. CanvasHeader.tsx:63,93 changes that rail from 4px to 8px vertical padding and from the small to medium control size, and the new fullscreen alignment regression depends on the resulting 28px control. Document those new rail metrics instead of claiming no change.
This paragraph still says every moveTab drop performs “empty-group removal,” but the reducer now intentionally preserves a source group when its last tab is moved (model.ts:570, with tests at model.test.ts:677-685). Update this contract to describe empty-group retention and explicit closing instead.
Verified the six observations embedded in review 5478488789. Follow-up fixes are in 6708377:
Confirmed and fixed: an adjacent node with an empty label changed from “Untitled” to “File” when added. The attachment now reuses the candidate's normalized display label, including missing/non-string label fallbacks. Added regression tests for all three cases.
Confirmed and fixed: empty Preview groups instructed users to double-click the Canvas even in fullscreen, where Canvas is unmounted. Empty groups now reuse the existing localized group-specific message regardless of focus/fullscreen state. Added unit coverage for both focus states and fullscreen modes, plus a browser assertion with Canvas actually unmounted.
Documentation corrected, layout preserved: composer padding is intentionally 12px top/left/right and 8px bottom, consistent with the newer equal-horizontal-padding contract. Updated the stale generic architecture text rather than reverting the UI.
Documentation corrected: the fullscreen rail is 48px wide, has 8px padding, and uses a 28px Layers disclosure button with a 16px icon; verified browser geometry.
Documentation corrected: moving a tab retains its empty source group. The drop handler then activates the destination. No move behavior was changed.
Not reproduced in Chromium; retained for compatibility follow-up: F2 focuses the native rename input, which is exposed in Chromium's accessibility tree as a non-ignored textbox named “Rename node”, even though its DOM ancestor is a tab. Text entry works. This does not constitute VoiceOver/NVDA verification. Per the agreed scope, the title editor structure remains unchanged.
Validation of this commit:
99 unit tests passed across the affected Chat-source and Preview-workspace suites.
4 isolated browser tests passed across preview-groups and chat-context-sources, including fullscreen empty-group coverage.
Repository pnpm typecheck and pnpm format passed.
Repository lint initially included pre-existing generated Playwright report assets; rerunning pnpm lint:fix --ignore-pattern 'apps/web/playwright-report/**' passed with 0 errors and the same 348 existing warnings. No lint/config exclusions were committed.
This branch has not been deployed
No deployments
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
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.
Summary
Review
Reviewed the complete original feature branch, then based this PR on the latest
origin/main. Earlier Layers, grouped dragging, Canvas chrome, and empty-chat disclaimer changes are already merged through #287 and are not repeated in this PR. The original branch is preserved.Review covered preview lifecycle transitions, batch-close scope and settlement, title-edit focus handling, empty-group persistence, and common UI consistency. Corrected stale test assertions for the expanded compact Canvas header's 44px height and the shared menu's compact padding. No remaining blocking correctness findings were identified.
Follow-up verification of Copilot's review confirmed and reproduced the two close-action focus findings in a browser; both are now fixed with unit and browser regressions. The missing
SelectedNodeRefsimport in the originally published snapshot is also repaired by the approved Chat context follow-up. The cross-group activation finding is a false positive for the current UI path: the drag handler callsactivateTabimmediately aftermoveTab, and Open to Side also activates the moved tab.Validation
pnpm typecheck: passed across all workspace packages.pnpm format: passed; no further tracked formatting changes.pnpm lint:fix: passed on the isolated snapshot with 348 existing warnings and no errors.git diff --check: passed.Limitations
Electron packaging and Windows-device behavior were not exercised in this review.
Follow-up review fixes (770fd9f)
Validation used an isolated exact snapshot: repository typecheck, format and lint passed (348 existing warnings); translation parity passed; 3,336 web unit tests were validated across 286 files (one inherited placeholder assertion was corrected and its full 9-test suite rerun). The isolated Vitest runner explicitly allowed the existing symlinked dependency asset directory. Expanded browser validation passed 48 of 51 tests, including all new review/PDF regressions. The three failures also reproduced on the unmodified b739efe baseline: Sketch type-button dragging, edge context-toolbar visibility, and the obsolete single stroke-toolbar selector. These baseline failures were not changed or hidden. Electron packaging and Windows-device behavior were not validated.