feat(dialog): migrate Manager dialog to Reka-UI (Phase 4)#12403
Open
dante01yoon wants to merge 1 commit into
Open
feat(dialog): migrate Manager dialog to Reka-UI (Phase 4)#12403dante01yoon wants to merge 1 commit into
dante01yoon wants to merge 1 commit into
Conversation
Flip useManagerDialog onto the Reka renderer added in Phase 0. Public API of useManagerDialog and useDialogService is unchanged. - dialogComponentProps.renderer='reka' + size='full' + Manager content sizing (matches legacy .manager-dialog: 1724px × 80vh, expanding to 2200×1320 above 3000px viewports). - modal=false to keep SingleSelect / SearchAutocomplete PrimeVue overlays working (focus trap + body pointer-events). Same reason Settings flipped to non-modal in Phase 3. - New useManagerDialog.test.ts pins the renderer flip against regression. Parent FE-571, this phase FE-576.
dosubot
Bot
added
the
size:S
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 1623 passed, 0 failed · 4 flaky📊 Browser Reports
|
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## jaewon/fe-575-dialog-reka-migration-phase-3 #12403 +/- ##
============================================================================
Coverage 59.90% 59.90%
============================================================================
Files 1415 1415
Lines 72289 72290 +1
Branches 20070 20070
============================================================================
+ Hits 43302 43307 +5
+ Misses 28513 28509 -4
Partials 474 474
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
dante01yoon
force-pushed
the
jaewon/fe-576-dialog-reka-migration-phase-4
branch
from
c0d46c6 to
3de88b7
Compare
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
2 participants
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
Phase 4 of the dialog migration. Flips
useManagerDialogonto the Reka renderer added in Phase 0, with content sizing that matches the legacy.manager-dialogCSS (1724px × 80vh, expanding to 2200×1320 above 3000px). Public API ofuseManagerDialog/useDialogServiceis unchanged.Parent: FE-571
This phase: FE-576
Predecessors: #11719 (Phase 0, merged), #12041 (Phase 1, merged), #12109 (Phase 2, merged), #12182 (Phase 3, stacked PR base)
Changes
src/workbench/extensions/manager/composables/useManagerDialog.ts(+12)renderer'reka'size'full'DialogContentto take the contentClass dimensionscontentClassw-[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.manager-dialogglobal CSS exactlymodalfalseSingleSelect,SearchAutocomplete-host scope, sort dropdown) teleported to body. Reka modal trap disables their pointer-events. Same fix Phase 3 applied to SettingsIntentionally left for Phase 6: the global
.manager-dialogCSS inGlobalDialog.vue<style>and the matchingclass="manager-dialog"onBaseModalLayout. Removing them here is a cascade-order risk (they currently overrideBaseModalLayoutsize="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,initialTabforwarding,initialPackIdforwarding,hide()closes.Verification
DOM probes (local dev,
useManagerDialog().show()against ComfyUI on :8189)[role="dialog"]at z-1804, rect 1724×794 —contentClassapplied exactlyBODY > DIV > listbox, z-3000 — escapes Rekaoverflow-hiddenComboboxPortal(z-3000,position="popper") — same teleport guarantee by constructionScreenshots from local verification will be attached as PR comments.
Quality gates
pnpm typecheck— cleanpnpm lint— clean for touched filespnpm format— appliedpnpm test:unit(touched + adjacent):useManagerDialog.test.ts— 5/5src/workbench/extensions/manager/composables/— 156/156useSettingsDialog.test.ts+src/components/dialog/(Phase 3 regression net) — 86/86Public API impact
None.
useManagerDialog().show(initialTab?, initialPackId?)keeps the same signature.Out of scope (later phases)
ConfirmDialogcallers — Phase 5 (FE-577)Dialog/<style>overrides inGlobalDialog.vue(incl..manager-dialog) — Phase 6 (FE-578)Review focus
contentClassmirrors the existing.manager-dialogCSS 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.modal: falserationale — same as Phase 3 Settings: Manager's PrimeVue-overlay children break under Reka modal focus trap. Acceptance Configure vite to copy from src to dist #2 ("Install/uninstall flows behave identically") is preserved because no overlay component changed; only the outer dialog renderer..manager-dialogCSS rule +class="manager-dialog"onBaseModalLayoutare kept on purpose. They overrideBaseModalLayout size="lg"'s 1280px cap with 1724px on small viewports; removing them naively regresses width. Phase 6 will replace viasizeprop or new variant.screenshot
Test plan
overflow-hidden