Copilot commented on code in PR #8581:
URL: https://github.com/apache/texera/pull/8581#discussion_r4052470240
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -566,7 +566,24 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
// without loading anything, so a request that then fails cannot strand
the visitor on
// an error instead of the page they would have gotten.
if (!this.config.env.formViewEnabled) {
- void this.router.navigate([USER_WORKSPACE, String(wid)], { replaceUrl:
true });
+ void this.router.navigateByUrl(workspaceCanvasUrl(wid), { replaceUrl:
true });
+ return;
+ }
+ // Arriving from the operator canvas of this same workflow, which kept its
session for us.
+ // The graph is already here and already in its co-editing room, so there
is nothing to fetch
+ // and nothing to rebuild: take the name and access from what is open and
settle in.
+ if (this.workflowActionService.hasWorkflowOpen(wid)) {
+ const metadata = this.workflowActionService.getWorkflowMetadata();
+ this.workflowName = metadata.name;
+ this.storedPositions = {
...(this.workflowActionService.getWorkflow().content?.operatorPositions ?? {})
};
+ this.canEdit = !metadata.readonly;
+ this.settleIntoForm();
Review Comment:
On a routed handoff during an active run, the retained
`ExecuteWorkflowService` still has the current state, but this component starts
`executionState` at `Uninitialized` and subscribes only to its non-replaying
`Subject`. Because this branch skips the reconnect that would emit another
state, the form can show **Run** instead of **Stop**, and entering authoring
mode can unlock the graph mid-run. Initialize from `getExecutionState()` before
settling in and add a mid-run handoff regression test.
##########
frontend/src/app/workspace/component/workspace.component.ts:
##########
@@ -316,6 +333,22 @@ export class WorkspaceComponent implements AfterViewInit,
OnInit, OnDestroy {
registerLoadOperatorMetadata() {
const wid = this.route.snapshot.params.id;
+ // The Form View handed this workflow over still open: the graph, its
co-editing room and the
+ // computing unit connection are the ones it was using. Nothing to fetch,
nothing to rebuild.
+ // Only the lock it put on the graph is undone, since editing is what the
canvas is for, and
+ // the view is centred because this canvas's paper is a new one, at its
default offset.
+ if (this.resumedSession) {
+ this.workflowActionService.enableWorkflowModification();
Review Comment:
A running/paused execution is intentionally handed over with the graph lock
disabled, but this unconditional call re-enables modification.
`ExecuteWorkflowService` only reapplies its lock when the execution state
changes, so a mid-run return to the canvas leaves operators editable until the
next state event. Reapply the service's current state-to-lock policy here
(enabling only idle/terminal states) instead of always enabling, and cover the
mid-run handoff.
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -205,8 +205,14 @@ export class WorkflowEditorComponent implements OnInit,
AfterViewInit, OnDestroy
}
ngAfterViewInit() {
- this.editor = document.getElementById("workflow-editor")!;
- this.editorWrapper = document.getElementById("workflow-editor-wrapper")!;
+ // This component's own elements, not whichever the document happens to
hold first. Two of
+ // these editors are briefly in the page at once when the two views of a
workflow hand over:
+ // the arriving one initialises while the departing one is still being
removed. Searching the
+ // document returned the departing view's container, so the paper was
built into a div about
+ // to disappear and the arriving canvas stayed blank, with nothing to pan
and nothing to click.
+ const host = this.elementRef.nativeElement as HTMLElement;
+ this.editor = host.querySelector("#workflow-editor")!;
+ this.editorWrapper = host.querySelector("#workflow-editor-wrapper")!;
Review Comment:
The host-scoped lookup fixes the destination, but routed switching now
remounts this component repeatedly while `ngOnDestroy` never calls
`this.paper.remove()`. Each old JointJS paper therefore remains subscribed to
the root-owned graph and retains a detached DOM tree, causing work and memory
to grow with every switch; `MiniMapComponent` has the same undisposed paper.
Dispose both papers during teardown before making remounts the normal switch
path.
##########
frontend/src/app/workspace/component/menu/menu.component.ts:
##########
@@ -756,15 +756,16 @@ export class MenuComponent implements OnInit, OnDestroy {
}
/**
- * The full-page handover to the Form View, apart from the save so the order
is testable.
- * Excluded from coverage as a whole: jsdom cannot navigate, so the specs
stub this method and
- * assert when it is called rather than what it does.
+ * The hand-over to the Form View, apart from the save so the order is
testable.
+ *
+ * A route, not a page load: the two views are views of one open workflow,
and reloading threw
+ * away everything that made the workflow live -- the shared document, the
computing unit
+ * connection, the execution state -- only to rebuild it on the other side.
The canvas keeps
+ * the session on its way out (see its ngOnDestroy) and the Form View
attaches to it.
*/
- /* v8 ignore start */
private openFormViewPage(wid: number): void {
- window.location.href = `${USER_WORKSPACE}/${wid}/form`;
+ void this.router.navigateByUrl(workspaceFormUrl(wid));
Review Comment:
This in-process route makes editor teardown/remount happen on every switch,
but `WorkflowEditorComponent` registers keydown with
`this._handleKeyboardAction.bind(this)` and removes a newly bound function, so
the original document listener is never removed. After returning to the canvas,
one Ctrl/Cmd-Z is handled by both stale and current instances and can undo
multiple entries. Store one bound callback and use that exact reference for
both `addEventListener` and `removeEventListener`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
