refactor(ui): merge nodes & workflow slices #7892
Merged
Conversation
This file contains 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
psychedelicious
requested review from
blessedcoolant,
maryhipp and
hipsterusername
as code owners
|
Because this targets a branch other than |
psychedelicious
force-pushed
the
psyche/feat/ui/workflow-touched
branch
from
3ff40b0 to
689bd2f
Compare
This allows undo/redo history to apply to node editor and workflow details/form.
psychedelicious
force-pushed
the
psyche/refactor/ui/merge-nodes-workflow-slices
branch
from
835ca64 to
b82a639
Compare
maryhipp
approved these changes
Apr 7, 2025
psychedelicious
deleted the
psyche/refactor/ui/merge-nodes-workflow-slices
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Demo of the improved undo functionality. Note the undo/redo working in form builder, and the "unsaved changes" indicator updating as changes are made (albeit with a short, intentional delay):
Screen.Recording.2025-04-07.at.8.23.22.pm.mov
One problem. I can't find a straightforward way to prevent the user's currently workflow form from being lost on first load after this change is deployed. Unsaved changes are lost, but the user can re-loading the saved workflow restore the form.
This change merges all logic from
workflowSlice(workflow name, description, form, etc) intonodesSlice(nodes and edges only).Ideally, we would write a
nodesSlicemigration that gets the oldworkflowSlicedata and adds it tonodesSliceon first load of this change. There is an API for slice migrations, but it's not sufficient for this sitaution.The redux persistence library doesn't give us access to all state when rehydrating a slice from browser storage. You only get the slice that is rehydrating. So in this case, the
nodesSlicemigration cannot see theworkflowSliceand therefore cannot copy the form and other workflow details into itself. The only option is to reset those things to their initial values.I'm sure it's possible but I'm not sure if it is worth the effort.
Related Issues / Discussions
This PR is based on #7891 and is motivated by some unpleasantries encountered while working on that change. It's also something that's been needing to be done for a while anyways.
QA Instructions
Besides the data loss issue, everything should work the same with better undo/redo action grouping.
Merge Plan
If we move ahead with this change, we should be sure to make a note in the release notes about the data loss.
Checklist
What's Newcopy (if doing a release after this PR)