Repository navigation
feat(canvas): add Auto-sort to the empty-canvas Tools submenu - #163
BIackFIame wants to merge 7 commits into
Conversation
…e startup bundle The canvas command palette and the canvas context menu are rendered only while they are open, but both were static imports of the canvas and so part of the startup bundle. They are now separate chunks, which frees about 18 KB of the startup bundle, and still open at once: the canvas preloads both chunks once it is idle after its first paint, and renders the loaded component directly. There is no lazy() and no Suspense boundary on this path (React holds a suspended boundary's reveal for up to 300 ms, which would delay the first right-click and the first Cmd/Ctrl+K). An open before the preload has finished loads the chunk at once and shows the menu as soon as it arrives. renderer-bundle-splitting checks that each is its own chunk, reachable only through a dynamic import(); canvas-preloaded-surfaces renders both with the real React through the real imports and checks that a preloaded menu and palette are in the very first render of an open.
Right-clicking empty canvas now shows a Tools row with one Auto-sort action. The criteria (by task, by status, by project, grid) are radio rows in that submenu, built with the existing canvas menu primitives. There is no separate panel, toolbar, presets or layouts list. - Scope: the current multi-selection when it holds two windows, otherwise terminal, plugin and Browser windows that are not completely inside a region. Companion mascots move only when selected. Sticky notes, materials and Files cards never move. - "By task" uses the orchestrator-to-subagent relation that already exists (parentSessionId): each subagent goes to the right of its parent. - The layout is deterministic: ties follow the current reading order, and sorting twice changes nothing. The block keeps the windows' top-left corner and keeps clear of HOME, regions, notes, materials, Files cards and windows it does not move (it shifts right or down, whichever is shorter, and never lands inside a region). With snapping on, every window lands on the 10px grid with the 20px snapping gap. - Moved windows get a presentation-only outline that clears on the next press or after a few seconds (they are not made the marquee selection), and a short glide that is skipped when reduced motion is requested. The Browser card does not glide, because its native view is placed from the measured card. - "Undo auto-sort" stays in the submenu while a sorted window is still where the sort put it. It restores only those windows. There is no global undo shortcut, so terminals and text inputs keep their keys. - The last criterion is kept in renderer memory for the running session, only after a sort happened. Nothing is persisted. A failed load or sort shows a localized toast. - The group-drag commit is extracted into commitLayerBounds and shared with auto-sort. - The Tools submenu body, scope and layout are not in the startup bundle: they load while the empty-canvas menu is open (or when Tools is pressed first), so the submenu still opens at once. Adds the official Lucide wrench and undo-2 icons, ru/en strings, and the UI contract text in en/ru/zh-CN. Tests drive the menu and the submenu.
…ed out - A context menu opened near a corner panel (minimap, shortcut hints, Home/zoom controls) moves up until it clears it; it was drawn over the bottom of the Home/zoom bar. When moving up cannot clear every panel (a tall menu near the right edge at UI scale 1.25, between the minimap and the shortcut hints), it moves sideways beside the panel it hits, at its own height; with no room on either side it keeps its place. Menus stay above the corner panels. - A submenu (Tools, Launch agent) opens where it covers no corner panel either: when its usual side and direction would run over the shortcut hints, minimap or Home/zoom controls (in whichever corners Settings put them), it opens in the other direction or on the other side; at UI scale 1.25 near the right edge, Tools opened over the hints panel and left its clipped text showing beside it. - The chevrons of Launch agent and Tools point to the side their submenu opens (the vendored chevron, rotated), not down. - "Needs at least 2 windows" is a plain hint, not the uppercase group label. It lines up with the radio labels and is as muted as the disabled rows it explains, so it does not read like an active item. - Zoomed out (summary), the outline of the windows a sort moved grows with the summary scale, so it stays visible at overview zoom for its 4 s. The placement code (and the existing chooseCanvasSubmenuSide) moves from canvasContextRouting to canvasMenuPlacement, which only the lazily loaded menu imports, so it stays out of the startup bundle.
…ffect ran When the idle preload settled after an open's render read the surface but before its effect ran, the effect saw a loaded surface and returned without re-rendering, so the first context menu or command palette could stay blank. The effect now re-renders whenever the render it follows had nothing to show.
…menu A menu first rendered while the Tools body was still loading started with no body; if the load finished before its effect ran (or before it became the empty-canvas menu), the effect and Tools both saw the module-level copy and skipped loading, so Tools expanded with no submenu. Both now copy it in.
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. |
![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: 65706b0d05
ℹ️ 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".
| }); | ||
| if (!plan) return; | ||
| host.commit(plan.after); | ||
| host.setState({ mode, undo: { mode, ...plan } }); |
There was a problem hiding this comment.
When the already-selected criterion is invoked again before anything moves, planCanvasAutoSort is idempotent, so the new plan has value-equal before and after maps. Replacing host.state.undo with it makes canvasAutoSortUndoTargets empty and removes “Undo auto-sort”, discarding the still-valid positions from the first sort even though all of those windows remain untouched; preserve the prior undo when the new plan changes no bounds (and skip the no-op commit/mark).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, confirmed with a test: running the same criterion again on an already sorted layout replaced the undo step with an equal before/after pair, so Undo auto-sort disappeared. Fixed in 62e95fb: a sort that moves nothing now skips the commit and the outline and keeps the earlier undo step. If a different criterion turns out to be a no-op, it only becomes the checked row and the undo step stays.
| const selected = movable.filter((candidate) => selection.has(candidate.id)); | ||
| if (selected.length >= 2) return selected.map((candidate) => candidate.id); | ||
| return movable | ||
| .filter((candidate) => !candidate.decorative) |
There was a problem hiding this comment.
When a marquee contains exactly one mascot (or a mascot plus only annotations), selected.length is below two and this fallback removes every decorative candidate unconditionally. With two regular windows present, Auto-sort therefore rearranges the regular windows but leaves the explicitly selected mascot behind; with only one regular window it is disabled altogether. Exclude unselected mascots here, but retain decorative candidates whose id is in selection.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed. A selection with one mascot (or a mascot plus notes) fell back to the default scope, which dropped every mascot, including the selected one. Fixed in 10a13b1: the fallback now leaves out only mascots that are not selected. This matches the UI contract ("companion mascots move only when selected"). Tests cover the scope function and the menu case of one window plus a selected mascot.
Choosing a criterion the layout already follows produced a plan whose before and after were equal. Storing it as the undo step dropped the previous sort's positions and hid Undo auto-sort. Such a sort now commits and outlines nothing and keeps the earlier undo step.
A selection with fewer than two windows falls back to the default scope, which left out every mascot, including one the person had just selected. A lone selected mascot was then left behind, or auto-sort was disabled next to a single regular window. The fallback now skips only mascots that are not selected, as the UI contract describes.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
This replaces #136 (reworked on top of current main after #158 and alongside #159; the old PR is closed with a link here).
Branch
pr/canvas-auto-sort-v2(head65706b0d, 5 commits), based onmain0fa6895d.Merge order: independent. It merges without conflicts with
main, #159, the reworked orchestration / isolation / subagent-worktree stack and the coming plugin locale/scale PR, before or after any of them.What changed since #136
ba427487ontomainafter Fix Codex prompt delivery and account-scoped quota status #158, with no conflicts.src/renderer/src/lib/preloadedSurface.ts. It now carries this PR's exact file, so whichever lands first, the other merges cleanly.Why
Your review of #136 kept only the auto-sort idea and listed what it should be:
Everything else from #136 is gone: snapshots, presets, broadcast input, the Tools panel, output search, the drop preview and the earlier layers.
What changes for users
lazy()/Suspense(whose reveal throttle would hold the first open for about 300 ms).main.Commits
cc2fa064perf(canvas): keep the command palette and the context menu out of the startup bundle71f33dd4feat(canvas): add Auto-sort to the empty-canvas Tools submenu720abc10fix(canvas): keep the Tools menu readable near corner panels and zoomed outf22aac54fix(canvas): show a preloaded menu that finished loading before its effect ran65706b0dfix(canvas): copy an already-loaded Tools submenu into a new context menuCommit 1 is independent and can be taken alone; commit 4 fixes it. Commit 3 also changes the placement of the existing Launch agent submenu.
Verification
macOS: this exact tree (
65706b0d), with a fake HOME. Results:build:helpers --host, typecheck, build andaudit:secretspass. The full suite gives 2277 / 2273 / 0 fail / 4 skipped (a first run had one failure inbrowser-engine-tabs, the known load flake that fix: fifteen bug fixes in isolation, orchestration, UI, plugins, GitHub sign-in and tests #159 fixes in that test; it passed on the rerun). The startup bundle is 1,676,591 bytes, which is 12,811 bytes smaller thanmain(1,689,402).Tests:
canvas-preloaded-surfacesrenders through the real React and react-dom/server and the real dynamic imports. It checks that:In
canvas-auto-sort, the Tools body is ready by the first render after Tools.renderer-bundle-splittingstill checks that both are their own chunks, reachable only throughimport().Combined:
main+ fix: fifteen bug fixes in isolation, orchestration, UI, plugins, GitHub sign-in and tests #159 + the reworked stack + this PR + the plugin locale/scale PR give the same tree in either merge order. The full suite on that tree gives 2627 / 2621 / 0 fail / 6 skipped.Linux (Ubuntu 24.04 VM, xvfb, earlier head, same markup and styles): screenshots of every criterion, the outline, undo, the disabled hint and submenu placement, in ru/en, sage/night, at UI scale 0.85, 1.0 and 1.25.
First open on Linux (VM, xvfb, CDP input, integration build, 3 launches × 2 combos, earlier head):
main5.1–8.8 ms);main;Not verified: Windows; the lilac palette.
Risks
Depends on
Nothing.