Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Confirm delete workflow #1772

Merged
merged 18 commits into from
Dec 4, 2024
Merged

Confirm delete workflow #1772

merged 18 commits into from
Dec 4, 2024

Conversation

webfiltered
Copy link
Contributor

@webfiltered webfiltered commented Dec 3, 2024

Add workflow confirm delete

From context menu:

image

Confirm:

image

Setting - on by default:

image

Overwrite file dialog converted:

image

Close unsaved workflow converted:

image

@webfiltered webfiltered requested a review from huchenlei December 3, 2024 07:19
@webfiltered
Copy link
Contributor Author

@huchenlei Can't seem to change the target of require() options using ConfirmPopup. Any idea what's going on there? It seems auto-bound to whatever was clicked, and ignores the setting completely.

Copy link
Member

@huchenlei huchenlei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's break this PR further down to following steps:

  • Define a global confirmation dialog component that replaces the usage of ComfyAsyncDialog:
    const res = (await ComfyAsyncDialog.prompt({
    title: 'Overwrite existing file?',
    message: `"${newPath}" already exists. Do you want to overwrite it?`,
    actions: ['Yes', 'No']
    })) as 'Yes' | 'No'
  • Trigger the global dialog when the command is invoked.

This method does not require usage of PrimeVue's confirm popup component. Overall I think the floating dialog is more suitable for this kind of command confirmation, as it is not binded to specific way of interaction. Let's say the we invoke the command with keyboard, we would still want to see the confirmation, but there won't be a intuitive element to bind the popup.

@webfiltered webfiltered force-pushed the confirm-delete-workflow branch from fc99f89 to ac9d5b3 Compare December 4, 2024 12:34
@webfiltered webfiltered force-pushed the confirm-delete-workflow branch from 5886add to 8931796 Compare December 4, 2024 18:41
@webfiltered webfiltered marked this pull request as ready for review December 4, 2024 19:04
@webfiltered webfiltered requested a review from huchenlei December 4, 2024 19:06
@huchenlei huchenlei merged commit 7351538 into main Dec 4, 2024
9 checks passed
@huchenlei huchenlei deleted the confirm-delete-workflow branch December 4, 2024 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Option to enable confirmation when deleting a workflow
2 participants