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

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

Merged
merged 75 commits into from
Dec 9, 2022
Merged
Show file tree
Hide file tree
Changes from 62 commits
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
027e143
✨ Added history store and mixin
MiloradFilipovic Nov 29, 2022
3a1aa22
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Nov 29, 2022
fe7d27d
✨ Implemented node position change undo/redo
MiloradFilipovic Nov 29, 2022
ff352cd
✨ Implemented move nodes bulk command
MiloradFilipovic Nov 29, 2022
6e9fbfb
⚡ Not clearing the redo stack after pushing the bulk command
MiloradFilipovic Nov 29, 2022
d17845b
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Nov 29, 2022
ad1cfea
🔨 Implemented commands using classes
MiloradFilipovic Nov 29, 2022
6b6ce4e
🔥 Removed unnecessary interfaces and actions
MiloradFilipovic Nov 29, 2022
45bae25
🔥 Removing unused constants
MiloradFilipovic Nov 30, 2022
ddded3b
🔨 Refactoring classes file
MiloradFilipovic Nov 30, 2022
b0b35fa
⚡ Adding eventBus to command obects
MiloradFilipovic Nov 30, 2022
fda6e8f
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Nov 30, 2022
8ff1b63
✨ Added undo/redo support for adding and removing nodes
MiloradFilipovic Nov 30, 2022
2f724da
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Nov 30, 2022
568be3f
✨ Implemented initial add/remove connections undo support
MiloradFilipovic Nov 30, 2022
b20f72e
⚡ Covering some corner cases with reconnecting nodes
MiloradFilipovic Nov 30, 2022
a73b875
⚡ Adding undo support for reconnecting nodes
MiloradFilipovic Nov 30, 2022
35f732c
⚡ Fixing going back and forward between undo and redo
MiloradFilipovic Nov 30, 2022
7c22278
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 1, 2022
601a0d3
✨ Implemented async command revert
MiloradFilipovic Dec 1, 2022
cb003e2
⚡ Preventing push to undo if bulk redo/undo is in progress
MiloradFilipovic Dec 1, 2022
316fb77
⚡ Handling re-connecting nodes and stopped pushing empty bulk actions…
MiloradFilipovic Dec 1, 2022
22f71e2
✨ Handling adding a node between two connected nodes
MiloradFilipovic Dec 1, 2022
bb008f7
⚡ Handling the case of removing multiple connections on the same inde…
MiloradFilipovic Dec 1, 2022
25827eb
⚡ Removing unnecessary timeouts, adding missing awaits, refactoring
MiloradFilipovic Dec 1, 2022
dc167da
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 1, 2022
8abd54c
⚡ Resetting history when opening new workflow, fixing incorrect bulk …
MiloradFilipovic Dec 2, 2022
cc6444d
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 2, 2022
87ff5f0
✔️ Fixing lint error
MiloradFilipovic Dec 2, 2022
2137c21
⚡ Minor refactoring + some temporary debugging logs
MiloradFilipovic Dec 2, 2022
5f211aa
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 2, 2022
f0dc35b
⚡ Preserving node properties when undoing it's removal, removing some…
MiloradFilipovic Dec 2, 2022
3e6477a
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 5, 2022
504107b
✨ Added undo/redo support for import workflow and node enable/disable
MiloradFilipovic Dec 5, 2022
4ef36bd
🔥 Removing some unused constant
MiloradFilipovic Dec 5, 2022
fe17735
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 5, 2022
e28da7e
✨ Added undo/redo support for renaming nodes
MiloradFilipovic Dec 5, 2022
253bb3b
⚡ Fixing rename history recording
MiloradFilipovic Dec 5, 2022
cb7bda2
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 5, 2022
9e88a72
✨ Added undo/redo support for duplicating nodes
MiloradFilipovic Dec 5, 2022
4a00b1c
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 5, 2022
52970b3
📈 Implemented telemetry events
MiloradFilipovic Dec 6, 2022
e215654
🔨 A bit of refactoring
MiloradFilipovic Dec 6, 2022
f1907f2
⚡ Fixing edgecases in removing connection and moving nodes
MiloradFilipovic Dec 6, 2022
cd5edea
⚡ Handling case of adding duplicate nodes when going back and forward…
MiloradFilipovic Dec 6, 2022
69313f1
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 6, 2022
d7720e1
⚡ Recording connections added directly to store
MiloradFilipovic Dec 6, 2022
c040209
⚡ Moving main history reset after wf is opened
MiloradFilipovic Dec 6, 2022
20eb771
🔨 Simplifying rename recording
MiloradFilipovic Dec 6, 2022
7030c21
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 6, 2022
6c6054c
📈 Adding NDV telemetry event, updating existing event name case
MiloradFilipovic Dec 6, 2022
c44951a
📈 Updating telemetry events
MiloradFilipovic Dec 7, 2022
791af5e
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 7, 2022
47e013a
⚡ Fixing duplicate connections on undo/redo
MiloradFilipovic Dec 7, 2022
1229ec7
⚡ Stopping undo events from firing constantly on keydown
MiloradFilipovic Dec 7, 2022
7efe3e2
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 7, 2022
48174f2
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 7, 2022
cd075f8
📈 Updated telemetry event for hitting undo in NDV
MiloradFilipovic Dec 7, 2022
0e3760a
⚡ Adding undo support for disabling nodes using keyboard shortcuts
MiloradFilipovic Dec 7, 2022
8f8bc57
⚡ Preventing adding duplicate connection commands to history
MiloradFilipovic Dec 7, 2022
0f5a2d5
⚡ Clearing redo stack when new change is added
MiloradFilipovic Dec 7, 2022
f590c5c
⚡ Preventing adding connection actions to undo stack while redoing them
MiloradFilipovic Dec 8, 2022
35d8ac3
👌 Addressing PR comments part 1
MiloradFilipovic Dec 8, 2022
a099612
👌 Moving undo logic for disabling nodes to `NodeView`
MiloradFilipovic Dec 8, 2022
b5a8468
👌 Implemented command comparing logic
MiloradFilipovic Dec 8, 2022
db98b94
⚡ Fix for not clearing redo stack on every user action
MiloradFilipovic Dec 9, 2022
f56d1bb
⚡ Fixing recording when moving nodes
MiloradFilipovic Dec 9, 2022
34d25f2
⚡ Fixing undo for moving connections
MiloradFilipovic Dec 9, 2022
68ac78c
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 9, 2022
6ae8f07
⚡ Fixing tracking new nodes after latest merge
MiloradFilipovic Dec 9, 2022
a159929
⚡ Fixing broken bulk delete
MiloradFilipovic Dec 9, 2022
7f3980f
Merge branch 'master' into feature/undo-redo
MiloradFilipovic Dec 9, 2022
386c6f6
⚡ Preventing undo/redo when not on main node view tab
MiloradFilipovic Dec 9, 2022
e244fce
👌 Addressing PR comments
MiloradFilipovic Dec 9, 2022
5e329e3
👌 Addressing PR comment
MiloradFilipovic Dec 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/editor-ui/src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ import { useUsersStore } from './stores/users';
import { useRootStore } from './stores/n8nRootStore';
import { useTemplatesStore } from './stores/templates';
import { useNodeTypesStore } from './stores/nodeTypes';
import { historyHelper } from '@/mixins/history';

export default mixins(
showMessage,
userHelpers,
restApi,
globalLinkActions,
historyHelper,
).extend({
name: 'App',
components: {
Expand Down Expand Up @@ -185,7 +187,6 @@ export default mixins(
this.loading = false;

this.trackPage();
// TODO: Un-comment once front-end hooks are updated to work with pinia store
this.$externalHooks().run('app.mount');

if (this.defaultLocale !== 'en') {
Expand Down
11 changes: 10 additions & 1 deletion packages/editor-ui/src/Interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,11 @@ import {
INodeCredentials,
INodeListSearchItems,
NodeParameterValueType,
IConnection,
MiloradFilipovic marked this conversation as resolved.
Show resolved Hide resolved
} from 'n8n-workflow';
import { FAKE_DOOR_FEATURES } from './constants';
import {ICredentialsDb} from "n8n";
import { BulkCommand, Undoable } from '@/models/history';

export * from 'n8n-design-system/src/types';

Expand Down Expand Up @@ -163,7 +166,7 @@ export interface IUpdateInformation {
export interface INodeUpdatePropertiesInformation {
name: string; // Node-Name
properties: {
[key: string]: IDataObject;
[key: string]: IDataObject | XYPosition;
};
}

Expand Down Expand Up @@ -1290,6 +1293,12 @@ export interface CurlToJSONResponse {
"parameters.sendBody": boolean;
}

export interface HistoryState {
redoStack: Undoable[];
undoStack: Undoable[];
currentBulkAction: BulkCommand | null;
bulkInProgress: boolean;
}
export type Basic = string | number | boolean;
export type Primitives = Basic | bigint | symbol;

Expand Down
28 changes: 26 additions & 2 deletions packages/editor-ui/src/components/Node.vue
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ import { workflowHelpers } from '@/mixins/workflowHelpers';
import { pinData } from '@/mixins/pinData';

import {
IDataObject,
MiloradFilipovic marked this conversation as resolved.
Show resolved Hide resolved
INodeTypeDescription,
ITaskData,
NodeHelpers,
Expand All @@ -117,13 +118,14 @@ import mixins from 'vue-typed-mixins';

import { get } from 'lodash';
import { getStyleTokenValue, getTriggerNodeServiceName } from '@/utils';
import { IExecutionsSummary, INodeUi, XYPosition } from '@/Interface';
import { IExecutionsSummary, INodeUi, INodeUpdatePropertiesInformation, XYPosition } from '@/Interface';
import { debounceHelper } from '@/mixins/debounce';
import { mapStores } from 'pinia';
import { useUIStore } from '@/stores/ui';
import { useWorkflowsStore } from '@/stores/workflows';
import { useNDVStore } from '@/stores/ndv';
import { useNodeTypesStore } from '@/stores/nodeTypes';
import { EnableNodeToggleCommand } from '@/models/history';

export default mixins(
externalHooks,
Expand Down Expand Up @@ -399,13 +401,17 @@ export default mixins(
}
},
mounted() {
this.$root.$on('enableNodeToggle', this.onRevertEnableToggle);
this.setSubtitle();
if (this.nodeRunData) {
setTimeout(() => {
this.$emit('run', {name: this.data && this.data.name, data: this.nodeRunData, waiting: !!this.waiting});
}, 0);
}
},
destroyed() {
this.$root.$off('enableNodeToggle', this.onRevertEnableToggle);
},
data () {
return {
isTouchActive: false,
Expand All @@ -417,6 +423,22 @@ export default mixins(
};
},
methods: {
onRevertEnableToggle({ nodeName, isDisabled }: { nodeName: string, isDisabled: boolean }) {
MiloradFilipovic marked this conversation as resolved.
Show resolved Hide resolved
const node: INodeUi|null = this.data;
if (node && nodeName === node.name) {
const updateInformation = {
name: node.name,
properties: {
disabled: isDisabled,
} as IDataObject,
} as INodeUpdatePropertiesInformation;

this.workflowsStore.updateNodeProperties(updateInformation);
this.workflowsStore.clearNodeExecutionData(node.name);
this.updateNodeParameterIssues(node);
this.updateNodeCredentialIssues(node);
}
},
showPinDataDiscoveryTooltip(dataItemsCount: number): void {
if (!this.isTriggerNode || this.isManualTypeNode || this.isScheduledGroup || dataItemsCount === 0) return;

Expand All @@ -433,7 +455,9 @@ export default mixins(
: nodeSubtitle;
},
disableNode () {
this.disableNodes([this.data]);
const nodeData = this.data as INodeUi;
MiloradFilipovic marked this conversation as resolved.
Show resolved Hide resolved
this.disableNodes([nodeData]);
this.historyStore.pushCommandToUndo(new EnableNodeToggleCommand(nodeData.name, !nodeData.disabled, nodeData.disabled === true, this));
this.$telemetry.track('User clicked node hover button', { node_type: this.data.type, button_name: 'disable', workflow_id: this.workflowsStore.workflowId });
},
executeNode () {
Expand Down
8 changes: 7 additions & 1 deletion packages/editor-ui/src/components/NodeSettings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ import { useUIStore } from '@/stores/ui';
import { useWorkflowsStore } from '@/stores/workflows';
import { useNDVStore } from '@/stores/ndv';
import { useNodeTypesStore } from '@/stores/nodeTypes';
import { useHistoryStore } from '@/stores/history';
import { RenameNodeCommand } from '@/models/history';

export default mixins(externalHooks, nodeHelpers).extend({
name: 'NodeSettings',
Expand All @@ -179,6 +181,7 @@ export default mixins(externalHooks, nodeHelpers).extend({
},
computed: {
...mapStores(
useHistoryStore,
useNodeTypesStore,
useNDVStore,
useUIStore,
Expand Down Expand Up @@ -498,6 +501,9 @@ export default mixins(externalHooks, nodeHelpers).extend({
this.$externalHooks().run('nodeSettings.credentialSelected', { updateInformation });
},
nameChanged(name: string) {
if (this.node) {
this.historyStore.pushCommandToUndo(new RenameNodeCommand(this.node.name, name, this));
}
// @ts-ignore
this.valueChanged({
value: name,
Expand Down Expand Up @@ -615,7 +621,7 @@ export default mixins(externalHooks, nodeHelpers).extend({
const updateInformation: IUpdateInformation = {
name: node.name,
value: nodeParameters,
};
} as IUpdateInformation;
MiloradFilipovic marked this conversation as resolved.
Show resolved Hide resolved

this.workflowsStore.setNodeParameters(updateInformation);

Expand Down
1 change: 1 addition & 0 deletions packages/editor-ui/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -430,4 +430,5 @@ export enum STORES {
VERSIONS = 'versions',
NODE_CREATOR = 'nodeCreator',
WEBHOOKS = 'webhooks',
HISTORY = 'history',
}
113 changes: 113 additions & 0 deletions packages/editor-ui/src/mixins/history.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import { useNDVStore } from '@/stores/ndv';
import { BulkCommand, Undoable } from '@/models/history';
import { useHistoryStore } from '@/stores/history';
import { useUIStore } from '@/stores/ui';
import { useWorkflowsStore } from '@/stores/workflows';
import { mapStores } from 'pinia';
import mixins from 'vue-typed-mixins';
import { Command } from '@/models/history';
import { debounceHelper } from '@/mixins/debounce';
import { deviceSupportHelpers } from '@/mixins/deviceSupportHelpers';
import Vue from 'vue';

const UNDO_REDO_DEBOUNCE_INTERVAL = 100;

export const historyHelper = mixins(debounceHelper, deviceSupportHelpers).extend({
computed: {
...mapStores(
useNDVStore,
useHistoryStore,
useUIStore,
useWorkflowsStore,
),
isNDVOpen(): boolean {
return this.ndvStore.activeNodeName !== null;
},
},
mounted() {
document.addEventListener('keydown', this.handleKeyDown);
},
destroyed() {
document.removeEventListener('keydown', this.handleKeyDown);
},
methods: {
handleKeyDown(event: KeyboardEvent) {
if (event.repeat) return;
if (this.isCtrlKeyPressed(event) && event.key === 'z') {
event.preventDefault();
if (!this.isNDVOpen) {
if (event.shiftKey) {
this.callDebounced('redo', { debounceTime: UNDO_REDO_DEBOUNCE_INTERVAL, trailing: true });
} else {
this.callDebounced('undo', { debounceTime: UNDO_REDO_DEBOUNCE_INTERVAL, trailing: true });
}
} else if (!event.shiftKey) {
this.trackUndoAttempt(event);
}
}
},
async undo() {
const command = this.historyStore.popUndoableToUndo();
if (!command) {
return;
}
if (command instanceof BulkCommand) {
this.historyStore.bulkInProgress = true;
const commands = command.commands;
const reverseCommands: Command[] = [];
for (let i = commands.length - 1; i >= 0; i--) {
await commands[i].revert();
reverseCommands.push(commands[i].getReverseCommand());
}
this.historyStore.pushUndoableToRedo(new BulkCommand(reverseCommands));
await Vue.nextTick();
this.historyStore.bulkInProgress = false;
}
if (command instanceof Command) {
await command.revert();
this.historyStore.pushUndoableToRedo(command.getReverseCommand());
this.uiStore.stateIsDirty = true;
}
this.trackCommand(command, 'undo');
},
async redo() {
const command = this.historyStore.popUndoableToRedo();
if (!command) {
return;
}
if (command instanceof BulkCommand) {
this.historyStore.bulkInProgress = true;
const commands = command.commands;
const reverseCommands = [];
for (let i = commands.length - 1; i >= 0; i--) {
await commands[i].revert();
reverseCommands.push(commands[i].getReverseCommand());
}
this.historyStore.pushBulkCommandToUndo(new BulkCommand(reverseCommands), false);
await Vue.nextTick();
this.historyStore.bulkInProgress = false;
}
if (command instanceof Command) {
await command.revert();
this.historyStore.pushCommandToUndo(command.getReverseCommand(), false);
this.uiStore.stateIsDirty = true;
}
this.trackCommand(command, 'redo');
},
trackCommand(command: Undoable, type: 'undo'|'redo'): void {
if (command instanceof Command) {
this.$telemetry.track(`User hit ${type}`, { commands_length: 1, commands: [ command.name ] });
} else if (command instanceof BulkCommand) {
this.$telemetry.track(`User hit ${type}`, { commands_length: command.commands.length, commands: command.commands.map(c => c.name) });
}
},
trackUndoAttempt(event: KeyboardEvent) {
if (this.isNDVOpen && !event.shiftKey) {
const activeNode = this.ndvStore.activeNode;
if (activeNode) {
this.$telemetry.track(`User hit undo in NDV`, { node_type: activeNode.type });
Copy link
Contributor

Choose a reason for hiding this comment

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

weird we are not tracking the event itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

}
}
},
},
});
10 changes: 9 additions & 1 deletion packages/editor-ui/src/mixins/nodeBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import { useWorkflowsStore } from "@/stores/workflows";
import { useNodeTypesStore } from "@/stores/nodeTypes";
import * as NodeViewUtils from '@/utils/nodeViewUtils';
import { getStyleTokenValue } from "@/utils";
import { useHistoryStore } from "@/stores/history";
import { MoveNodeCommand } from "@/models/history";

export const nodeBase = mixins(
deviceSupportHelpers,
Expand All @@ -33,6 +35,7 @@ export const nodeBase = mixins(
useNodeTypesStore,
useUIStore,
useWorkflowsStore,
useHistoryStore,
),
data (): INodeUi | null {
return this.workflowsStore.getNodeByName(this.name);
Expand Down Expand Up @@ -286,6 +289,7 @@ export const nodeBase = mixins(
// some dirty DOM query to get the new positions till I have more time to
// create a proper solution
let newNodePosition: XYPosition;
this.historyStore.startRecordingUndo();
moveNodes.forEach((node: INodeUi) => {
const element = document.getElementById(node.id);
if (element === null) {
Expand All @@ -304,9 +308,13 @@ export const nodeBase = mixins(
position: newNodePosition,
},
};

const oldPosition = node.position;
if (oldPosition[0] !== newNodePosition[0] || oldPosition[1] !== newNodePosition[1]) {
this.historyStore.pushCommandToUndo(new MoveNodeCommand(node.name, oldPosition, newNodePosition, this));
}
this.workflowsStore.updateNodeProperties(updateInformation);
MiloradFilipovic marked this conversation as resolved.
Show resolved Hide resolved
});
this.historyStore.stopRecordingUndo();

this.$emit('moved', node);
}
Expand Down
9 changes: 8 additions & 1 deletion packages/editor-ui/src/mixins/nodeHelpers.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { EnableNodeToggleCommand } from './../models/history';
import { useHistoryStore } from '@/stores/history';
import {
PLACEHOLDER_FILLED_AT_EXECUTION_TIME,
CUSTOM_API_CALL_KEY,
Expand Down Expand Up @@ -51,6 +53,7 @@ export const nodeHelpers = mixins(
computed: {
...mapStores(
useCredentialsStore,
useHistoryStore,
useNodeTypesStore,
useSettingsStore,
useWorkflowsStore,
Expand Down Expand Up @@ -432,12 +435,14 @@ export const nodeHelpers = mixins(
},

disableNodes(nodes: INodeUi[]) {
this.historyStore.startRecordingUndo();
for (const node of nodes) {
const oldState = node.disabled;
// Toggle disabled flag
const updateInformation = {
name: node.name,
properties: {
disabled: !node.disabled,
disabled: !oldState,
} as IDataObject,
} as INodeUpdatePropertiesInformation;

Expand All @@ -447,7 +452,9 @@ export const nodeHelpers = mixins(
this.workflowsStore.clearNodeExecutionData(node.name);
this.updateNodeParameterIssues(node);
this.updateNodeCredentialIssues(node);
this.historyStore.pushCommandToUndo(new EnableNodeToggleCommand(node.name, oldState === true, node.disabled === true, this));
}
this.historyStore.stopRecordingUndo();
},
// @ts-ignore
getNodeSubtitle (data, nodeType, workflow): string | undefined {
Expand Down
Loading