You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "chihumyum (via GitHub)" <gi...@apache.org> on 2026/09/19 11:26:44 UTC
[PR] refactor(desktop): move sharing dialog ownership below AppShell [maka]
chihumyum opened a new pull request, #5505:
URL: https://github.com/apache/maka/pull/5505
## Summary
The sharing dialog's target still lived in `AppShellContent`, while its UI called the Desktop bridge directly from a legacy file. `SessionCollaborationDialogRoot` now owns the target and dialog inside `features/session-collaboration`, with a registered controller owner and injected sharing/clipboard services. The shell receives only the stable opener, localized action label, and modal-open projection; target changes and dialog polling do not re-render the shell. AppShell hook call sites decrease from 61 to 60, and registered owners increase from six to seven.
The existing invitation, access-management, Turn-approval, and remote-access Settings flows remain intact. Changing the sharing target now starts a fresh dialog so an old invitation cannot appear under another Session. Redundant shell props, the legacy UI file, and unused public exports are removed.
This completes the **Session Collaboration dialog** sub-slice only. Session settings / Plan intent is being developed separately; the full Session controls checklist remains open. Refs #4582.
## Verification
- Node 24.19.0: final Desktop suite **2,750/2,750**, including **15/15** focused sharing, adapter, joining, and Guest-request tests.
- Full workspace build and typecheck; final renderer build; lint, format, Desktop/UI Knip, ASF headers, Windows inventory, AppShell hook gate, and Astryx inventory passed.
- Renderer architecture fixtures **112/112** and the final checker against `upstream/main` passed. Merge-tree against `3924c6557` is clean.
- Negative checks: removing the target key or shell-frame memoization fails the corresponding regression assertion; publicly exporting the controller is rejected by the ownership guard. All mutations were restored.
- No visual redesign. Native Electron end-to-end sharing and clipboard interaction were not exercised; behavior tests mount the production root/dialog with injected services.
## AI use
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: OpenAI Codex implemented the migration, tests, documentation, and validation under the contributor's direction. The commit carries `Generated-by: Codex`; independent human review is still required.
## Checklist
- [x] Tests cover the change and fail without it
- [x] Lint, format, typecheck and the affected suites pass locally
Does this PR entail a change in behavior?
- [x] Yes — target replacement clears the previous dialog's invitation and access state, as described above; existing sharing actions retain their semantics.
- [ ] No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor(desktop): move sharing dialog ownership below AppShell [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #5505:
URL: https://github.com/apache/maka/pull/5505
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor(desktop): move sharing dialog ownership below AppShell [maka]
Posted by "chihumyum (via GitHub)" <gi...@apache.org>.
chihumyum commented on PR #5505:
URL: https://github.com/apache/maka/pull/5505#issuecomment-5772336457
@Astro-Han Thanks for the earlier GO review. Could you please take another look at the rebased head `ce5d83fcf` against `main` at `e6db75689`?
The conflicts are resolved. The AppShell merge preserves main's current onboarding/bootstrap ownership while retaining the feature-owned sharing dialog and narrow shell projection. I regenerated the architecture ledger and Astryx inventory from the merged source; all six existing controller owners and the Session Collaboration owner remain registered. There were no inline findings to address in the earlier review.
Local validation passed with Node 24.19.0 / npm 11.19.0: clean build, typecheck, lint, formatting, strict-base renderer architecture (112 fixture tests), Desktop tests (2,814/2,814), Astryx/Windows inventories, AppShell hook gate, ASF headers, Knip, `git diff --check`, and merge-tree verification. Native Electron/clipboard acceptance was not rerun locally. Hosted checks must rerun for this new head.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor(desktop): move sharing dialog ownership below AppShell [maka]
Posted by "chihumyum (via GitHub)" <gi...@apache.org>.
chihumyum commented on PR #5505:
URL: https://github.com/apache/maka/pull/5505#issuecomment-5788717454
@Astro-Han Could you please take another look at rebased head `625a5ade7689ee0be151ea9e75222f303a87d6ac` against `main` at `518fd529c0a15a84e6fa5a13f903bf1f28701cd2`?
The new merge conflict is resolved. Only the generated renderer architecture ledger conflicted; I regenerated it and the Astryx inventory from the merged source. All existing controller owners and the Session Collaboration owner remain registered. The range-diff confirms that the PR's source changes are unchanged, with only the generated counts adjusted for current main. Merge-tree and `git diff --check` pass.
Local validation passed with Node 24.19.0 / npm 11.19.0: clean full build, workspace typecheck, Desktop tests (2,824/2,824), strict-base renderer architecture (112/112 fixtures), lint, formatting, Desktop/UI Knip, ASF headers, Windows/Astryx inventories, and the AppShell hook gate. Native Electron sharing and clipboard acceptance were not rerun. Hosted CI must validate this new head.
Automated update: posted by OpenAI Codex at the contributor's request. Independent human review is still required.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org