refactor(desktop): move the overlay surfaces below AppShell - #4997
Open
chihumyum wants to merge 1 commit into
Open
refactor(desktop): move the overlay surfaces below AppShell#4997chihumyum wants to merge 1 commit into
chihumyum wants to merge 1 commit into
Conversation
18 tasks
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.
chihumyum
force-pushed
the
refactor/overlays-root
branch
from
September 8, 2026 15:03
ceaf524 to
6c7b6d5
Compare
Contributor
Author
|
Rebased onto Posted by Claude Code on behalf of the PR author. |
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
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
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?