refactor(dialog): migrate to frappe-ui v1 Dialog API#480
Merged
Conversation
Aligns with the project convention of explicit `lucide-` prefixes for Lucide icons (also required by frappe-ui v1, which no longer falls back to FeatherIcon for unprefixed strings). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sweeping migration of every dialog call site in gameplan to the v1
Dialog surface shipped in frappe-ui (see frappe-ui v1-release/dialog).
Declarative <Dialog> rewrites:
- flatten :options="{ ... }" blob to top-level props across all dialogs
- replace #body-content / #body wrappers with default slot + bare mode
- swap manual focus hacks (v-focus, ref + setTimeout) for autofocus attr
- update action onClick from (close) => ... to ({ close }) => ... ctx form
- v-model -> v-model:open
Imperative API rewrites:
- migrate 16 destructive confirms (delete/discard/remove/stop/retract) to
dialog.danger, dropping redundant theme: 'red' / confirmLabel: 'Delete'
- adopt onConfirm callback contract with throw-to-stay-open semantics
- remove now-obsolete frontend/src/utils/dialogs.tsx wrapper (-93 lines)
Submodule:
- bump frappe-ui pointer to pick up the v1 imperative API helpers
39 files changed, +799 insertions, -1202 deletions (net -403 lines).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
gameplan
 |
||||||||||||||||||||||||||||||
| Project |
gameplan
|
| Branch Review |
v1-dialog
|
| Run status |
|
| Run duration | 00m 57s |
| Commit |
|
| Committer | Faris Ansari |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
0
|
 |
0
|
 |
0
|
 |
0
|
 |
7
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
…omboboxes
The v1 Dialog renders its body inside reka-ui's DialogPortal, which mounts
the content asynchronously. The helper's one-shot cy.get('body').then(...)
check could run before the input-mode trigger was in the DOM, falling
through to the wrong (button-mode) selector and timing out.
Switch to a retrying .should() that waits until either trigger form is
present before deciding which path to take.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
1 participant
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
Sweeping migration of every dialog call site in gameplan to the v1 Dialog surface shipped in
frappe-uiv1-release/dialog.Net impact: 39 files changed, +798 / -1202 lines (net -404 lines). One module deleted entirely (
frontend/src/utils/dialogs.tsx, -93 lines).What changed
Declarative
<Dialog>rewrites:options="{ ... }"blob to top-level props across all dialogs#body-content/#bodywrappers with default slot +baremodev-focus,ref+setTimeout) for the standardautofocusattributeonClickfrom(close) => ...to({ close }) => ...ctx formv-model→v-model:opendisableOutsideClickToClose→:dismissable="false"(inverted)Imperative API rewrites
dialog.danger, dropping redundanttheme: 'red'/confirmLabel: 'Delete'sincedangerprovides both as defaultsonConfirmcallback contract: auto-close on resolve, throw-to-stay-open with the thrown message rendered inline as a validation errorfrontend/src/utils/dialogs.tsxwrapper — the imperative helpers now ship fromfrappe-uidirectly viadialog.confirm/dialog.danger/dialog.promptSubmodule
frappe-uipointer to pick up the v1 imperative API helpers, the close-behaviour story, and the aligned docsTop files by churn
pages/Project.vuecomponents/CommandPalette/CommandPalette.vuebaremode +Dialog.Closere-exportcomponents/DiscussionView.vuedialog.dangercomponents/RevisionsDialog.vuecomponents/AddMemberDialog.vuecomponents/NewTaskDialog/NewTaskDialog.vuecomponents/Poll.vuedialog.danger(stop / retract / delete)components/NewSpaceDialog.vueutils/dialogs.tsxpages/NewDiscussion/useNewDiscussion.tsdialog.dangercomponents/AboutDialog.vueDestructive call sites migrated to
dialog.dangercomponents/TaskDetail.vuecomponents/TaskList.vuecomponents/Poll.vuecomponents/SpaceOptions.vuecomponents/Comment.vuecomponents/CommentsArea.vuecomponents/CommentsList.vuecomponents/DiscussionView.vuecomponents/Settings/Members.vuepages/ProjectDiscussionNew.vuepages/PageGrid.vuepages/Page.vuepages/NewDiscussion/useNewDiscussion.tsNon-destructive
dialog.confirmusages (archive, mark-as-read, close/re-open discussion, change role, save-or-discard) were intentionally left ondialog.confirm—dialog.dangerwould mis-colour those flows red.Related work in
frappe-uiPushed on
v1-release/dialog:acc50047—Imperative.vuestory: close-behaviour matrix,optimisticClose,keepOpenAfterAsyncdemos134aa37e— docs alignment: migration guide §7, spec 08f Imperative API section, ADR-0002 marked superseded, new ADR-0003 explaining why implementation reversed the original caller-closes designTest plan
dialog.dangerand show the red alert-triangle icon: delete task, delete space, delete page, delete poll, delete comment, delete discussion, delete draft, remove user, stop poll, retract voteNewSpaceDialogfirst field receives focus,NewTaskDialog/EditCategoryDialogetc. focus their primary input:dismissable="false"(in-flight forms)baremodev-model(legacy) still works wherever it remains🤖 Generated with Claude Code