refactor(desktop): move the overlay surfaces below AppShell - #4997
refactor(desktop): move the overlay surfaces below AppShell#4997chihumyum wants to merge 1 commit into
Conversation
ceaf524 to
6c7b6d5
Compare
|
Rebased onto Posted by Claude Code on behalf of the PR author. |
6c7b6d5 to
4b2b3d1
Compare
|
Rebased onto Posted by Claude Code on behalf of the PR author. |
|
@Astro-Han this one is ready for your look when you have time: the Commands / overlays slice claimed on #4582, sitting on Posted by Claude Code on behalf of the PR author. |
372e1f1 to
96340b4
Compare
|
For the record: the one red run on this PR (at Posted by Claude Code on behalf of the PR author. |
Give the keyboard help, the Command Palette, the Search modal and the Settings modal one owner outside the shell. `features/overlays` holds their open state, the Settings request with its sub-surfaces, the Search scroll target and the global shortcuts; `OverlaysRoot` is the registered owner of `useOverlaysController` and hands the shell frame the overlays through a render prop, so `AppShellContent` calls none of the four hooks. The legacy overlay layer reads what to show through `OverlaysConsumer` and keeps only the lazy Settings modal and the palette's command list, which are shell concerns: its props drop from 34 to 17. The thread search, the remembered Settings section and the focus settle before Settings opens reach the feature through one Desktop adapter. `AppShellContent` loses four hooks (40 to 36 in the gate inventory) and five legacy files leave the renderer root.
96340b4 to
02f5542
Compare
|
The Desktop E2E step is red again at
I am not going to keep force-pushing to re-roll the E2E; the next rebase (which Posted by Claude Code on behalf of the PR author. |
Summary
Move the shell's overlay surfaces below
AppShell: the keyboard help, the Command Palette, the Search modal, and the Settings modal now have one owner,features/overlays, withOverlaysRootregistered incontrollerOwnersas the only caller ofuseOverlaysController.OverlaysRootowns the four open flags, the Settings request with its three sub-surfaces (provider catalog, connection detail, provider create), the Search scroll target, and the global shortcuts (mod+k,mod+/,mod+?, bare?). It hands the shell frame the overlays through a render prop the wayTaskEntryRoothands overtaskEntry, soAppShellandAppShellContentcall none of the four hooks: the gate inventory goes from 40 hooks / 71 call sites to 36 / 67.openSettingsSurfaceapplies it, and the section an intent lands on is what the adapter persists; the transitions are unit-tested without React.openProjectSettingsstill replaces the whole request, and closing still keeps the connection slug and create type for the next open, as before.OverlaysConsumer; its props drop from 34 to 17, and the 15 pass-through values the shell used to thread into it are gone.use-shell-search.tswas the last renderer-root reader ofwindow.maka.search.keyboard-help.tsx,command-palette.tsx,command-palette-types.ts,use-shell-search.ts,use-settings-modal.ts, 251 → 246 legacy files). The ledger'scommands-and-overlaysownership entry now namesfeatures/overlaysas the home of the two files that stay.searchThreadthe Search modal's debounce depends on.useSessionCollaborationDialog, the fifth modal inhasModalOpen, which needsopenSettingsSectionfrom this slice and follows separately; and the palette's command list, which stays a shell concern because its rows are shell actions.Refs #4582
Verification
test:dist2415/2415,@maka/uiand@maka/desktoptypecheck (stories included), lint, format:checkcheck:renderer-architecture --base upstream/mainwith the newOverlaysRootowner registration passing the guard;check:app-shell-hooksat 36 / 67; Astryx inventory; Knip (apps/desktop); ASF headers;git diff --checkoverlays-model(9: every Settings intent, close, profile no-op, scroll-target consumption),overlays-provider-scope(5: closed initial projection, stable commands across opens, persisted section and single blur per closed-to-open, request replacement on a project open, scroll target handled once, service routing, the missing-root throw),overlays-boundary(8: services hook only in the controller, no bridge or shell dependency in the slice, the entry surface pinned to the shell / overlay layer / composition, adapter owned by composition, controller and fakes out of the entry,window.maka.searchabsent from every production module, oneOverlaysRootmount and overlay UI only in the overlay layer, no overlay hook left in the shell),overlays-services-adapter(2)Product/Command Searchstory mounts the palette throughOverlaysRootand opens it through the owner's commandsidebar-project-reloadasserts the Search modal, and the E2E fixture now opens it through the overlays command instead of the removed setterReview focus
The hand-off and the nesting.
OverlaysRootsits insideTaskEntryRoot's frame and aroundAppShellContent, so an overlay change re-renders the shell frame exactly as the shell's ownuseStatedid before, and nothing above it. The only remaining injection point iscommandOptions.AI use
Select exactly one:
Tool(s) and scope: Claude Code designed the slice, wrote the implementation and tests, and ran the verification; the human contributor reviewed the work and chose to submit it.
Checklist
Does this PR entail a change in behavior?