mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-07-16 16:58:33 +00:00
5a53df8d79
## Summary Phase 4 of the dialog migration. Flips `useManagerDialog` onto the Reka renderer added in Phase 0, with content sizing that matches the legacy `.manager-dialog` CSS (1724px × 80vh, expanding to 2200×1320 above 3000px). Public API of `useManagerDialog` / `useDialogService` is unchanged. Parent: [FE-571](https://linear.app/comfyorg/issue/FE-571/dialog-system-migration-primevue-reka-ui-parent) This phase: [FE-576](https://linear.app/comfyorg/issue/FE-576/phase-4-migrate-manager-dialog-designer-review) Predecessors: #11719 (Phase 0, merged), #12041 (Phase 1, merged), #12109 (Phase 2, merged), #12182 (Phase 3, **stacked PR base**) > **Stacked on Phase 3**: this PR targets `jaewon/fe-575-dialog-reka-migration-phase-3`. Rebase onto `main` after #12182 lands. ## Changes ### `src/workbench/extensions/manager/composables/useManagerDialog.ts` (+12) | Field | Value | Reason | | --- | --- | --- | | `renderer` | `'reka'` | Flip onto the new path | | `size` | `'full'` | Free `DialogContent` to take the contentClass dimensions | | `contentClass` | `w-[90vw] max-w-[1724px] sm:max-w-[1724px] h-[80vh] max-h-[1026px] min-[3000px]:max-w-[2200px] min-[3000px]:max-h-[1320px] rounded-2xl overflow-hidden` | Mirrors legacy `.manager-dialog` global CSS exactly | | `modal` | `false` | Manager hosts PrimeVue overlays (`SingleSelect`, `SearchAutocomplete`-host scope, sort dropdown) teleported to body. Reka modal trap disables their pointer-events. Same fix Phase 3 applied to Settings | Intentionally left for Phase 6: the global `.manager-dialog` CSS in `GlobalDialog.vue` `<style>` and the matching `class="manager-dialog"` on `BaseModalLayout`. Removing them here is a cascade-order risk (they currently override `BaseModalLayout` size="lg" 1280px cap with 1724px); Phase 6 owns the CSS-overrides cleanup pass. ### Tests - `src/workbench/extensions/manager/composables/useManagerDialog.test.ts` **(new)** — 5 tests: renderer flip + Manager sizing, non-modal, `initialTab` forwarding, `initialPackId` forwarding, `hide()` closes. ## Verification ### DOM probes (local dev, `useManagerDialog().show()` against ComfyUI on :8189) | Probe | Result | | --- | --- | | Manager dialog node | `[role="dialog"]` at z-1804, rect 1724×794 — `contentClass` applied exactly | | SingleSelect listbox | Teleported to `BODY > DIV > listbox`, z-3000 — escapes Reka `overflow-hidden` | | SearchAutocomplete | Uses Reka `ComboboxPortal` (`z-3000`, `position="popper"`) — same teleport guarantee by construction | | Stacked Reka confirm over Manager | Manager z-1804, confirm z-1806 — vRekaZIndex orders correctly | | ESC on stacked confirm | Top confirm closes, Manager remains open | Screenshots from local verification will be attached as PR comments. ## Quality gates - [x] `pnpm typecheck` — clean - [x] `pnpm lint` — clean for touched files - [x] `pnpm format` — applied - [x] `pnpm test:unit` (touched + adjacent): - `useManagerDialog.test.ts` — 5/5 - `src/workbench/extensions/manager/composables/` — 156/156 - `useSettingsDialog.test.ts` + `src/components/dialog/` (Phase 3 regression net) — 86/86 - [ ] CI Playwright matrix - [x] Manual verification on a backend (ComfyUI :8189) ## Public API impact None. `useManagerDialog().show(initialTab?, initialPackId?)` keeps the same signature. ## Out of scope (later phases) - `ConfirmDialog` callers — Phase 5 (FE-577) - Removing PrimeVue `Dialog`/`<style>` overrides in `GlobalDialog.vue` (incl. `.manager-dialog`) — Phase 6 (FE-578) - Designer pass on Manager dimensions (FE-576 acceptance #1) — owner Jaewon, async with this PR ## Review focus 1. **Sizing translated literally** — `contentClass` mirrors the existing `.manager-dialog` CSS rule (height + max-width + max-height + 3000px breakpoint). Net visible should be byte-identical to today. Worth a designer pass per FE-576 acceptance criteria. 2. **`modal: false` rationale** — same as Phase 3 Settings: Manager's PrimeVue-overlay children break under Reka modal focus trap. Acceptance #2 ("Install/uninstall flows behave identically") is preserved because no overlay component changed; only the outer dialog renderer. 3. **Phase 6 deferred cleanup** — `.manager-dialog` CSS rule + `class="manager-dialog"` on `BaseModalLayout` are kept on purpose. They override `BaseModalLayout size="lg"`'s 1280px cap with 1724px on small viewports; removing them naively regresses width. Phase 6 will replace via `size` prop or new variant. ## screenshot <img width="1440" height="828" alt="keybinding-panel" src="https://github.com/user-attachments/assets/c8e4ae15-860c-4f61-b48f-795f5e24912d" /> <img width="1440" height="828" alt="nested-modify-keybinding" src="https://github.com/user-attachments/assets/a6890183-be0f-4831-919e-4edb2ee2bb04" /> <img width="1920" height="992" alt="phase4-manager-node-pack-dropdown" src="https://github.com/user-attachments/assets/e4f97486-8b89-408a-9f83-92fbbfd5c29b" /> <img width="1440" height="828" alt="settings-dialog-reka" src="https://github.com/user-attachments/assets/7764ac51-703e-40a9-9ff8-af664b9ad11a" /> ## Test plan - [x] Unit: 5/5 new + 156/156 manager composables + 86/86 adjacent - [ ] CI: full Vitest + Playwright matrix - [x] Manual on a backend: - Open Manager (nav, search filter, install button visible, scroll grid) - SingleSelect (Node Pack) dropdown not clipped by dialog `overflow-hidden` - Stacked Reka confirm over Manager renders above with correct z-index - ESC closes only the top dialog --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: GitHub Action <action@github.com>