feat(editor): Add undo/redo support for canvas actions #4787

MiloradFilipovic · 2022-12-01T15:05:02Z

See comments this Linear ticket for more info about testing.

* master: fix: Update padding for resource filters dropdown (#4751) fix: Update workflow save button type and design and share button type (#4752) fix: Update owner badge padding (no-changelog) (#4749) fix: Remove background for resource ownership selector (#4748) fix: Update size of select components in filters dropdown (#4747)

* master: test: Skip e2e check of rendered nodes in node creator (#4761) feat: Add dynamic translations. Change how sharing unavailable is handled (no-changelog) (#4758) ci: Move full E2E tests into their own scheduled workflow (#4757) feat(Todoist Node): Update to use latest api version (#4650) fix(Google Sheets Node): Fix exception if no matching rows are found

* master: fix: Lazy load nodes for credentials testing (#4760) fix(core): Fix `$items().length` in Execute Once mode (#4755) feat(Google Calendar Node): Use resource locator component for calendar parameters (#4410) fix: Change the currentUserHasAccess flag behavior (no-changelog) (#4763)

* master: fix(core): Ensure executions list is properly filtered for all users (#4765) ci: Fix linting issues on master (no-changelog) (#4768) fix(Microsoft Outlook Node): Fix binary attachment upload (#4766)

* master: fix(editor): JSON view values can be mapped like keys (#4702) test: E2E NDV (#4712) fix: Issue building image with dependency name ending in .vue (no-changelog) (#4773) fix: Register community nodes as known nodes (no-changelog) (#4775) refactor: Codex updates for Item Lists, Markdown and Date & Time (#4772) refactor(editor): Patch ElementUI tooltip memory leak (#4769) refactor: Apply lint rule `cred-class-field-display-name-miscased` (#4762)

… to undo stack

…x. Adding debounce to undo/redo keyboard calls

mutdmour · 2022-12-08T12:15:50Z

packages/editor-ui/src/mixins/history.ts

+			if (this.isNDVOpen && !event.shiftKey) {
+				const activeNode = this.ndvStore.activeNode;
+				if (activeNode) {
+					this.$telemetry.track(`User hit undo in NDV`, { node_type: activeNode.type });


weird we are not tracking the event itself

Talked with Nik about it, looks like we are only interested in the fact that users attempted undo here.

packages/editor-ui/src/mixins/nodeBase.ts

mutdmour · 2022-12-08T12:21:15Z

packages/editor-ui/src/stores/history.ts

+	}),
+	getters: {
+		currentBulkContainsConnectionCommand() {
+			return (command: AddConnectionCommand): boolean => {


not sure what the exact problem here you are solving is.. but maybe this could be more generic.. if each class had a compareTo method like in Java or a toString representation, you can use that to check if an instance of that event exists in history

Now thinking about it, this may no longer be necessary. It has happened in some weird scenarios when going back and forward in history that the same connection command would end up in undo due to jsplump connections listeners so this prevents it. This may have stopped now that redo is cleared on each new user action which simplifies navigation options. Will have to check, but the generic approach is the way to go here for sure.

mutdmour · 2022-12-08T12:30:54Z

packages/editor-ui/src/views/NodeView.vue

+				this.suspendRecordingDetachedConnections = true;
+				this.__addConnection(connection, true);


what if you tracked history changes only in __addConnection instead of connection bind event? then you could pass trackHistory: false and not track events.. and not need this flag to suspend recoding events.. same question applies to other events..
not a huge fan of this flag because it seems flaky and easy thing to mess up...

I don't like it ether, but found out not all connections are added and removed ind __addConnection and removeConnection methods, some are added and removed in those events. For example, if you drag a connection on an empty canvas space, it is removed in the connectionDetached event (among other situations, hence the flag), also when reconnecting nodes, some connections are pushed straight to store in conncection event. This made implementation rely on those flags which is a bit of chasing my own tail situation but I couldn't figure out better way to cover all cases.

* master: fix: Use license-sdk v1.6.1 (#4872) feat(editor): Node creator actions (#4696) test(editor): Set e2e test retries (#4870) fix: Increase workflow reactivation max timeout to 1 day (#4869) fix: Stop returning `UNKNOWN ERROR` in the response if an actual error message is available (#4859) fix: Issue listing executions with Postgres (#4856) test(editor): Fix flaky e2e tests (#4779) fix: Use the same entrypoint for custom docker images as for the other images (no-changelog) (#4849) refactor: Deprecate `alwaysOpenEditWindow` for `string` (#4839) fix: Upgrade sse-channel to mitigate CVE-2019-10744 (#4835) # Conflicts: # packages/editor-ui/src/App.vue # packages/editor-ui/src/views/NodeView.vue

* master: fix: Update duplicate action (#4858)

mutdmour · 2022-12-09T12:40:34Z

packages/editor-ui/src/views/NodeView.vue

@@ -2620,7 +2620,7 @@ export default mixins(
 					return;
 				}

-				if (trackHistory) {
+				if (trackHistory && !partOfBulkDelete) {


why do we need this? should not it just be bundled as part of current bulk action?

Nope, bulk action inside this function is used when deleting a single connected node. In that case, it will contain removeNode and removeConnection commands. When deleting multiple nodes, this function is called in a loop and recording is started outside of this loop, so starting and stopping recording in each iteration will break the history recording.

packages/editor-ui/src/views/NodeView.vue

mutdmour · 2022-12-09T12:59:45Z

packages/editor-ui/src/views/NodeView.vue

@@ -2620,7 +2620,7 @@ export default mixins(
 					return;
 				}

-				if (trackHistory && !partOfBulkDelete) {
+				if (trackHistory && !trackBulk) {


nitpick - now it's confusing.. I would change the default to true as trackBulk should track bulk actions..

Suggested change

if (trackHistory && !trackBulk) {

if (trackHistory && trackBulk) {

Ah yeah, makes sense.

* master: fix(editor): Avoid adding manual trigger node when webhook node is added (#4887) ci: Add libc6-compat to nightly docker builds (no-changelog) (#4885) fix(Move Binary Data Node): Stringify objects before encoding them in MoveBinaryData (#4882) ci: Upgrade pnpm and turborepo (no-changelog) (#4878) fix(editor): Fix for broken tab navigation (#4881) feat(editor): Add undo/redo support for canvas actions (#4787) fix(Split In Batches Node): Fix issue with pairedItem (#4873) fix: Update duplicate action (#4858) # Conflicts: # packages/editor-ui/src/Interface.ts # packages/editor-ui/src/mixins/history.ts # packages/editor-ui/src/views/NodeView.vue

janober · 2022-12-12T16:39:34Z

Got released with n8n@0.207.0

MiloradFilipovic added 24 commits November 29, 2022 10:37

✨ Added history store and mixin

027e143

✨ Implemented node position change undo/redo

fe7d27d

✨ Implemented move nodes bulk command

ff352cd

⚡ Not clearing the redo stack after pushing the bulk command

6e9fbfb

🔨 Implemented commands using classes

ad1cfea

🔥 Removed unnecessary interfaces and actions

6b6ce4e

🔥 Removing unused constants

45bae25

🔨 Refactoring classes file

ddded3b

⚡ Adding eventBus to command obects

b0b35fa

✨ Added undo/redo support for adding and removing nodes

8ff1b63

Merge branch 'master' into feature/undo-redo

2f724da

* master: fix(core): Ensure executions list is properly filtered for all users (#4765) ci: Fix linting issues on master (no-changelog) (#4768) fix(Microsoft Outlook Node): Fix binary attachment upload (#4766)

✨ Implemented initial add/remove connections undo support

568be3f

⚡ Covering some corner cases with reconnecting nodes

b20f72e

⚡ Adding undo support for reconnecting nodes

a73b875

⚡ Fixing going back and forward between undo and redo

35f732c

✨ Implemented async command revert

601a0d3

⚡ Preventing push to undo if bulk redo/undo is in progress

cb003e2

⚡ Handling re-connecting nodes and stopped pushing empty bulk actions…

316fb77

… to undo stack

✨ Handling adding a node between two connected nodes

22f71e2

⚡ Handling the case of removing multiple connections on the same inde…

bb008f7

…x. Adding debounce to undo/redo keyboard calls

MiloradFilipovic self-assigned this Dec 1, 2022

n8n-assistant bot added n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels Dec 1, 2022

⚡ Removing unnecessary timeouts, adding missing awaits, refactoring

25827eb

MiloradFilipovic changed the title ~~WIP: Feature/undo redo~~ feat (editor): Add undo/redo support for canvas actions Dec 1, 2022

MiloradFilipovic marked this pull request as draft December 1, 2022 20:39

MiloradFilipovic added 3 commits December 7, 2022 16:41

⚡ Preventing adding duplicate connection commands to history

8f8bc57

⚡ Clearing redo stack when new change is added

0f5a2d5

⚡ Preventing adding connection actions to undo stack while redoing them

f590c5c

mutdmour reviewed Dec 8, 2022

View reviewed changes

MiloradFilipovic added 3 commits December 8, 2022 14:58

👌 Addressing PR comments part 1

35d8ac3

👌 Moving undo logic for disabling nodes to NodeView

a099612

👌 Implemented command comparing logic

b5a8468

MiloradFilipovic requested a review from mutdmour December 8, 2022 15:02

MiloradFilipovic added 8 commits December 9, 2022 10:07

⚡ Fix for not clearing redo stack on every user action

db98b94

⚡ Fixing recording when moving nodes

f56d1bb

⚡ Fixing undo for moving connections

34d25f2

⚡ Fixing tracking new nodes after latest merge

6ae8f07

⚡ Fixing broken bulk delete

a159929

Merge branch 'master' into feature/undo-redo

7f3980f

* master: fix: Update duplicate action (#4858)

⚡ Preventing undo/redo when not on main node view tab

386c6f6

mutdmour reviewed Dec 9, 2022

View reviewed changes

packages/editor-ui/src/views/NodeView.vue Outdated Show resolved Hide resolved

👌 Addressing PR comments

e244fce

MiloradFilipovic requested a review from mutdmour December 9, 2022 12:50

mutdmour reviewed Dec 9, 2022

View reviewed changes

👌 Addressing PR comment

5e329e3

MiloradFilipovic requested a review from mutdmour December 9, 2022 13:06

mutdmour approved these changes Dec 9, 2022

View reviewed changes

MiloradFilipovic merged commit b2aba48 into master Dec 9, 2022

MiloradFilipovic deleted the feature/undo-redo branch December 9, 2022 14:07

n8n-assistant bot added the Upcoming Release Will be part of the upcoming release label Dec 9, 2022

janober removed the Upcoming Release Will be part of the upcoming release label Dec 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(editor): Add undo/redo support for canvas actions #4787

feat(editor): Add undo/redo support for canvas actions #4787

MiloradFilipovic commented Dec 1, 2022 •

edited

Loading

mutdmour Dec 8, 2022

MiloradFilipovic Dec 8, 2022

mutdmour Dec 8, 2022

MiloradFilipovic Dec 8, 2022

mutdmour Dec 8, 2022

MiloradFilipovic Dec 8, 2022

mutdmour Dec 9, 2022

MiloradFilipovic Dec 9, 2022

mutdmour Dec 9, 2022

mutdmour Dec 9, 2022

MiloradFilipovic Dec 9, 2022

janober commented Dec 12, 2022

		this.suspendRecordingDetachedConnections = true;
		this.__addConnection(connection, true);

	if (trackHistory && !trackBulk) {
	if (trackHistory && trackBulk) {

feat(editor): Add undo/redo support for canvas actions #4787

feat(editor): Add undo/redo support for canvas actions #4787

Conversation

MiloradFilipovic commented Dec 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janober commented Dec 12, 2022

MiloradFilipovic commented Dec 1, 2022 •

edited

Loading