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

Update state when graph cleared via UI #88

Merged

Conversation

christian-byrne
Copy link
Collaborator

ChangeTracker is not tracking the graph being cleared via the clear-workflow buttons in the UI. Probably because clicks on the alert dialog don't trigger a state check. This is another iteration of this issue.

Here's the situation that can result from this:

  1. The user accidentally clicks the clear-workflow button (and clicks confirm accidentally, or has the confirm dialog disabled via settings)
  2. The user tries to Ctrl+Z undo, and it doesn't work.
  3. The user believes that that workflow is now lost, so they close the program, load a new workflow, etc.

In reality, the user would just need to do something else that would trigger a state check. But on first attempt, it will fail.

This change solves the issue by dispatching a graphCleared event to the api when the clear-workflow button is clicked in the old or new menu, and has ChangeTracker listen for that event.

I think it's maybe an unusual thing for the api to serve as the observer for such a thing, but that seems to be the pattern right now until there's a more clear dilineation that separates the event systems.

@huchenlei
Copy link
Member

@pythongosssss Can you take a look at this?

@huchenlei huchenlei requested a review from pythongosssss July 5, 2024 12:53
@huchenlei huchenlei added the bug Something isn't working label Jul 5, 2024
@pythongosssss pythongosssss merged commit 27c5bc1 into Comfy-Org:main Jul 5, 2024
4 checks passed
huchenlei pushed a commit that referenced this pull request Jul 6, 2024
Rename

Remove ts-nocheck, fix errors

Update state when graph cleared via UI (#88)

Convert to ts, fix imports
huchenlei added a commit that referenced this pull request Jul 6, 2024
Rename

Remove ts-nocheck, fix errors

Update state when graph cleared via UI (#88)

Convert to ts, fix imports

Co-authored-by: pythongosssss <[email protected]>
@christian-byrne christian-byrne deleted the update-state-on-clear branch July 6, 2024 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants