Skip to content

fix(gui): enable frontend undo-redo with bug-fix for shared editing #3836

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Oct 10, 2025
Merged

fix(gui): enable frontend undo-redo with bug-fix for shared editing #3836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
55b4779
Revert "fix(ui): temporarily disable frontend undo and redo (#3571)"
Xiao-zhen-Liu Oct 6, 2025
2fb1a9e
fix(gui): enable frontend undo-redo with bugfix for shared editing
Xiao-zhen-Liu Oct 7, 2025
5d343ac
fix(gui): enable frontend undo-redo with bugfix for shared editing
Xiao-zhen-Liu Oct 7, 2025
cddba96
Merge branch 'main' into xiaozhen-fix-undoredo
Yicong-Huang Oct 8, 2025
cd31cca
Merge branch 'main' into xiaozhen-fix-undoredo
Xiao-zhen-Liu Oct 8, 2025
ed3b1c5
refactoring.
Xiao-zhen-Liu Oct 9, 2025
0d48c73
Merge branch 'main' into xiaozhen-fix-undoredo
Xiao-zhen-Liu Oct 10, 2025
7d99f3d
Merge branch 'main' into xiaozhen-fix-undoredo
Xiao-zhen-Liu Oct 10, 2025
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
32 changes: 16 additions & 16 deletions core/gui/src/app/workspace/component/menu/menu.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,22 +276,22 @@
nzType="database"
nzTheme="twotone"></i>
</button>
<!-- <button-->
<!-- (click)="undoRedoService.undoAction()"-->
<!-- [disabled]="displayParticularWorkflowVersion || !undoRedoService.canUndo()"-->
<!-- nz-button>-->
<!-- <i-->
<!-- nz-icon-->
<!-- nzType="undo"></i>-->
<!-- </button>-->
<!-- <button-->
<!-- (click)="undoRedoService.redoAction()"-->
<!-- [disabled]="displayParticularWorkflowVersion || !undoRedoService.canRedo()"-->
<!-- nz-button>-->
<!-- <i-->
<!-- nz-icon-->
<!-- nzType="redo"></i>-->
<!-- </button>-->
<button
(click)="undoRedoService.undoAction()"
[disabled]="displayParticularWorkflowVersion || !undoRedoService.canUndo()"
nz-button>
<i
nz-icon
nzType="undo"></i>
</button>
<button
(click)="undoRedoService.redoAction()"
[disabled]="displayParticularWorkflowVersion || !undoRedoService.canRedo()"
nz-button>
<i
nz-icon
nzType="redo"></i>
</button>
</nz-button-group>
</ng-template>
<nz-dropdown-menu #menu="nzDropdownMenu">
Expand Down
100 changes: 49 additions & 51 deletions core/gui/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,56 +897,54 @@ describe("WorkflowEditorComponent", () => {
expect(jointGraphWrapper.getCurrentHighlightedOperatorIDs()).toContain(mockResultPredicate.operatorID);
});

// Temporarily disabling undo-redo because of a bug that can cause invalid workflow structures.
// TODO: enable after fixing the bug.
// //undo
// it("should undo action when user presses command + Z or control + Z", () => {
// spyOn(workflowVersionService, "getDisplayParticularVersionStream").and.returnValue(of(false));
// spyOn(undoRedoService, "canUndo").and.returnValue(true);
// let undoSpy = spyOn(undoRedoService, "undoAction");
// fixture.detectChanges();
// const commandZEvent = new KeyboardEvent("keydown", { key: "Z", metaKey: true, shiftKey: false });
// (document.activeElement as HTMLElement)?.blur();
// document.dispatchEvent(commandZEvent);
// fixture.detectChanges();
// expect(undoSpy).toHaveBeenCalledTimes(1);
//
// const controlZEvent = new KeyboardEvent("keydown", { key: "Z", ctrlKey: true, shiftKey: false });
// (document.activeElement as HTMLElement)?.blur();
// document.dispatchEvent(controlZEvent);
// fixture.detectChanges();
// expect(undoSpy).toHaveBeenCalledTimes(2);
// });
//
// //redo
// it("should redo action when user presses command/control + Y or command/control + shift + Z", () => {
// spyOn(workflowVersionService, "getDisplayParticularVersionStream").and.returnValue(of(false));
// spyOn(undoRedoService, "canRedo").and.returnValue(true);
// let redoSpy = spyOn(undoRedoService, "redoAction");
// fixture.detectChanges();
// const commandYEvent = new KeyboardEvent("keydown", { key: "y", metaKey: true, shiftKey: false });
// (document.activeElement as HTMLElement)?.blur();
// document.dispatchEvent(commandYEvent);
// fixture.detectChanges();
// expect(redoSpy).toHaveBeenCalledTimes(1);
//
// const controlYEvent = new KeyboardEvent("keydown", { key: "y", ctrlKey: true, shiftKey: false });
// (document.activeElement as HTMLElement)?.blur();
// document.dispatchEvent(controlYEvent);
// fixture.detectChanges();
// expect(redoSpy).toHaveBeenCalledTimes(2);
//
// const commandShitZEvent = new KeyboardEvent("keydown", { key: "z", metaKey: true, shiftKey: true });
// (document.activeElement as HTMLElement)?.blur();
// document.dispatchEvent(commandShitZEvent);
// fixture.detectChanges();
// expect(redoSpy).toHaveBeenCalledTimes(3);
//
// const controlShitZEvent = new KeyboardEvent("keydown", { key: "z", ctrlKey: true, shiftKey: true });
// (document.activeElement as HTMLElement)?.blur();
// document.dispatchEvent(controlShitZEvent);
// fixture.detectChanges();
// expect(redoSpy).toHaveBeenCalledTimes(4);
// });
//undo
it("should undo action when user presses command + Z or control + Z", () => {
spyOn(workflowVersionService, "getDisplayParticularVersionStream").and.returnValue(of(false));
spyOn(undoRedoService, "canUndo").and.returnValue(true);
let undoSpy = spyOn(undoRedoService, "undoAction");
fixture.detectChanges();
const commandZEvent = new KeyboardEvent("keydown", { key: "Z", metaKey: true, shiftKey: false });
(document.activeElement as HTMLElement)?.blur();
document.dispatchEvent(commandZEvent);
fixture.detectChanges();
expect(undoSpy).toHaveBeenCalledTimes(1);

const controlZEvent = new KeyboardEvent("keydown", { key: "Z", ctrlKey: true, shiftKey: false });
(document.activeElement as HTMLElement)?.blur();
document.dispatchEvent(controlZEvent);
fixture.detectChanges();
expect(undoSpy).toHaveBeenCalledTimes(2);
});

//redo
it("should redo action when user presses command/control + Y or command/control + shift + Z", () => {
spyOn(workflowVersionService, "getDisplayParticularVersionStream").and.returnValue(of(false));
spyOn(undoRedoService, "canRedo").and.returnValue(true);
let redoSpy = spyOn(undoRedoService, "redoAction");
fixture.detectChanges();
const commandYEvent = new KeyboardEvent("keydown", { key: "y", metaKey: true, shiftKey: false });
(document.activeElement as HTMLElement)?.blur();
document.dispatchEvent(commandYEvent);
fixture.detectChanges();
expect(redoSpy).toHaveBeenCalledTimes(1);

const controlYEvent = new KeyboardEvent("keydown", { key: "y", ctrlKey: true, shiftKey: false });
(document.activeElement as HTMLElement)?.blur();
document.dispatchEvent(controlYEvent);
fixture.detectChanges();
expect(redoSpy).toHaveBeenCalledTimes(2);

const commandShitZEvent = new KeyboardEvent("keydown", { key: "z", metaKey: true, shiftKey: true });
(document.activeElement as HTMLElement)?.blur();
document.dispatchEvent(commandShitZEvent);
fixture.detectChanges();
expect(redoSpy).toHaveBeenCalledTimes(3);

const controlShitZEvent = new KeyboardEvent("keydown", { key: "z", ctrlKey: true, shiftKey: true });
(document.activeElement as HTMLElement)?.blur();
document.dispatchEvent(controlShitZEvent);
fixture.detectChanges();
expect(redoSpy).toHaveBeenCalledTimes(4);
});
});
});
32 changes: 15 additions & 17 deletions core/gui/src/app/workspace/component/workflow-editor/workflow-editor.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,23 +190,21 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy
.pipe(takeUntil(this._onProcessKeyboardActionObservable))
.subscribe(displayParticularWorkflowVersion => {
if (!displayParticularWorkflowVersion) {
// Temporarily disabling undo-redo because of a bug that can cause invalid workflow structures.
// TODO: enable after fixing the bug.
// // cmd/ctrl+z undo ; ctrl+y or cmd/ctrl + shift+z for redo
// if ((event.metaKey || event.ctrlKey) && !event.shiftKey && event.key.toLowerCase() === "z") {
// // UNDO
// if (this.undoRedoService.canUndo()) {
// this.undoRedoService.undoAction();
// }
// } else if (
// ((event.metaKey || event.ctrlKey) && !event.shiftKey && event.key.toLowerCase() === "y") ||
// ((event.metaKey || event.ctrlKey) && event.shiftKey && event.key.toLowerCase() === "z")
// ) {
// // redo
// if (this.undoRedoService.canRedo()) {
// this.undoRedoService.redoAction();
// }
// }
// cmd/ctrl+z undo ; ctrl+y or cmd/ctrl + shift+z for redo
if ((event.metaKey || event.ctrlKey) && !event.shiftKey && event.key.toLowerCase() === "z") {
// UNDO
if (this.undoRedoService.canUndo()) {
this.undoRedoService.undoAction();
}
} else if (
((event.metaKey || event.ctrlKey) && !event.shiftKey && event.key.toLowerCase() === "y") ||
((event.metaKey || event.ctrlKey) && event.shiftKey && event.key.toLowerCase() === "z")
) {
// redo
if (this.undoRedoService.canRedo()) {
this.undoRedoService.redoAction();
}
}
// below for future hotkeys
}
this._onProcessKeyboardActionObservable.complete();
Expand Down
36 changes: 33 additions & 3 deletions core/gui/src/app/workspace/service/workflow-graph/model/shared-model-change-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,12 @@ export class SharedModelChangeHandler {
event.changes.keys.forEach((change, key) => {
if (change.action === "add") {
const newLink = this.texeraGraph.sharedModel.operatorLinkMap.get(key) as OperatorLink;
const jointLinkCell = JointUIService.getJointLinkCell(newLink);
jointElementsToAdd.push(jointLinkCell);
linksToAdd.push(newLink);
// Validate the link first
if (this.validateAndRepairNewLink(newLink)) {
const jointLinkCell = JointUIService.getJointLinkCell(newLink);
jointElementsToAdd.push(jointLinkCell);
linksToAdd.push(newLink);
}
}
if (change.action === "delete") {
keysToDelete.push(key);
Expand Down Expand Up @@ -200,6 +203,33 @@ export class SharedModelChangeHandler {
});
}

/**
* Check the sanity of a newly added link. We have constraints on a new link (it should connect to operators and
* ports that exist, and it should not be duplicated with another link connecting to the same operator ports.) Such
* constraints are enforced if the change to the shared model comes from local UI (`WorkflowGraph.addLink()`). If
* the change is initiated by the `UndoManager` or from remote collaborators, however, due to the limitations of Yjs,
* it is not possible to check the sanity of this operation before it is applied to the shared model. To ensure the
* integrity of the shared model, we validate the link add operation here instead, and repair the shared model if it
* violates the constraints.
* @param newLink A new link that has already been added to the shared model
* @returns Whether this new link passes the sanity check. If it does, this change can be applied to the UI. Otherwise
* this link is already deleted from the shared model.
*/
private validateAndRepairNewLink(newLink: OperatorLink): boolean {
try {
this.texeraGraph.assertLinkNotDuplicated(newLink);
// Verify the link connects to operators and ports that exist.
this.texeraGraph.assertLinkIsValid(newLink);
return true;
} catch (error) {
// Invalid link, repair the shared model
this.texeraGraph.sharedModel.operatorLinkMap.delete(newLink.linkID);
// This is treated as a normal repair step and not an error.
console.log("failed to add link. cause: ", (error as Error).message);
return false;
}
}

/**
* Syncs element positions. Will temporarily block local updates.
* @private
Expand Down
13 changes: 13 additions & 0 deletions core/gui/src/app/workspace/service/workflow-graph/model/workflow-graph.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1111,6 +1111,19 @@ export class WorkflowGraph {
}
}

/**
* Checks if a link is unique in the graph. Throws an error if more than one link with the same source and target
* as the given link exists.
*/
public assertLinkNotDuplicated(link: OperatorLink): void {
const links = this.getAllLinks().filter(
value => isEqual(value.source, link.source) && isEqual(value.target, link.target)
);
if (links.length > 1) {
throw new Error(`duplicate link found with same source and target as link ${link}`);
}
}

/**
* Retrieves a subgraph (subDAG) from the workflow graph. This method excludes disabled operators and links.
*
Expand Down
Loading