mengw15 commented on code in PR #8391:
URL: https://github.com/apache/texera/pull/8391#discussion_r3939294319
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -133,25 +157,164 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
this.workflowActionService.disableWorkflowModification();
}
+ /**
+ * Size the name field to its text, the way the operator canvas does, so
what follows
+ * it starts at the same place in both views instead of after a fixed-width
box.
+ */
+ private adjustWorkflowNameWidth(): void {
+ const input =
this.host.nativeElement.querySelector<HTMLInputElement>("input.wf-name");
+ if (!input) {
+ return;
+ }
+ /* v8 ignore start -- font-metrics DOM measuring; jsdom has no layout */
+ const probe = document.createElement("span");
+ probe.style.visibility = "hidden";
+ probe.style.position = "absolute";
+ probe.style.whiteSpace = "pre";
+ probe.style.font = getComputedStyle(input).font;
+ probe.textContent = input.value || input.placeholder;
+ document.body.appendChild(probe);
+ input.style.width = `${Math.min(probe.offsetWidth + 20, 800)}px`;
+ document.body.removeChild(probe);
+ /* v8 ignore stop */
+ }
+
+ private refreshSavedState(): void {
+ const lastModified =
this.workflowActionService.getWorkflowMetadata()?.lastModifiedTime;
+ this.autoSaveState =
+ lastModified === undefined
+ ? ""
+ : "Saved at " +
+ (this.datePipe.transform(
+ lastModified,
+ "MM/dd/yyyy HH:mm:ss",
+ Intl.DateTimeFormat().resolvedOptions().timeZone,
+ "en"
+ ) ?? "");
+ }
+
+ /**
+ * Renaming here is the same edit as renaming on the operator canvas: commit
the name and
+ * save. The title bar itself -- the read-back (normalised) name and its
width -- is
+ * refreshed from the metadata subscription below, the single place this
page's own rename
+ * and a co-editor's both flow through.
+ */
+ public onRenameWorkflow(): void {
+ this.workflowActionService.setWorkflowName(this.workflowName);
+ this.save();
+ }
+
+ /**
+ * Keep the title bar in step with the workflow's metadata, exactly as the
operator canvas
+ * does: a rename or a save -- this page's own or a co-editor's -- refreshes
the name, its
+ * width, and the "Saved at ..." state from one place, so the two views
never drift apart.
+ */
+ private registerMetadataRefresh(): void {
+ this.workflowActionService
+ .workflowMetaDataChanged()
+ // The same 100ms the operator canvas debounces its title-bar refresh by.
+ .pipe(debounceTime(100), untilDestroyed(this))
+ .subscribe(() => {
+ this.workflowName =
this.workflowActionService.getWorkflowMetadata()?.name ?? "";
+ this.later(() => this.adjustWorkflowNameWidth(), 0);
+ this.refreshSavedState();
+ });
+ }
+
/**
* Switch to the operator canvas with a full page load, not a route. The two
views share
* root-level singletons (the graph, the Yjs shared model, the CU
connection); handing
* over in-process left the old state attached -- undraggable operators, a
ghost coeditor
* of yourself, broken runs. A fresh document is the reliable handover.
*/
public openRegularCanvas(): void {
+ this.save();
/* v8 ignore start -- full-document navigation; jsdom cannot navigate */
window.location.href = `${USER_WORKSPACE}/${this.wid}`;
/* v8 ignore stop */
}
+ /**
+ * Save the same way the operator canvas does. Both views edit one workflow,
so the
+ * form has to write through the same debounced persist -- otherwise an
author's
+ * setup, or a value someone filled in, would be gone on the next visit.
+ */
+ private registerAutoPersist(): void {
+ this.workflowActionService
+ .workflowChanged()
+ .pipe(debounceTime(SAVE_DEBOUNCE_TIME_IN_MS), untilDestroyed(this))
+ .subscribe(() => this.save());
+ }
+
+ /**
+ * Save the workflow this page opened, and only that one. The persist
endpoint creates a
+ * workflow when the payload has no id, so saving whatever the graph holds
would spawn
+ * stray "Untitled workflow" rows when the page is left before its workflow
loaded.
+ */
+ private save(): void {
+ if (!this.userService.isLogin() ||
!this.workflowPersistService.isWorkflowPersistEnabled()) {
+ return;
+ }
+ const workflow = this.workflowActionService.getWorkflow();
+ if (workflow.wid === undefined || workflow.wid !== this.wid) {
+ return;
+ }
+ const preserved: Workflow = {
+ ...workflow,
+ content: { ...workflow.content, operatorPositions:
this.positionsToSave(workflow.content) },
+ };
+ // On the way out the subscription must NOT be tied to this component:
ngOnDestroy
+ // calls save(), and untilDestroyed would tear the subscription down as
part of the
+ // very same destroy sequence, aborting the request that was the point of
the call.
+ const persist = this.workflowPersistService.persistWorkflow(preserved);
+ // The `destroyed` branch deliberately omits untilDestroyed (see above);
the persist call
+ // is a one-shot HTTP request that completes on its own, so it needs no
teardown operator.
+ // eslint-disable-next-line rxjs-angular/prefer-takeuntil
+ (this.destroyed ? persist : persist.pipe(untilDestroyed(this))).subscribe({
+ next: () => this.refreshSavedState(),
+ // A save that fails silently is the worst thing this page can do: the
author walks
+ // away believing the form they just built is stored.
+ error: () => this.notificationService.error("Could not save. Your latest
changes are not stored yet."),
+ });
+ }
+
+ /**
+ * A position for every operator (stored, else the graph's current, else
origin). Loading
+ * throws on an operator with no position, so a partial map would make the
workflow
+ * unopenable -- which is what writing the stored map wholesale did for any
newer operator.
+ */
+ private positionsToSave(content: WorkflowContent): { [operatorID: string]:
Point } {
+ const positions: { [operatorID: string]: Point } = {};
+ for (const operator of content.operators) {
+ positions[operator.operatorID] =
this.storedPositions[operator.operatorID] ??
Review Comment:
`getWorkflowContent` reads positions from the shared `elementPositionMap` —
data written by whoever drags, not anything this page measures — so
`content.operatorPositions` is live here, a co-editor's canvas moves included,
while `storedPositions` is the load-time snapshot. Preferring the snapshot
means every save from this page (the exit save included) writes those moves
back to where things sat when the page opened — the #8315 stale-copy overwrite,
from the page that means to be read-only over the graph. And an operator added
since load already falls through to the live map, so the snapshot preference is
inconsistent with itself. `content.operatorPositions?.[id] ?? stored ?? origin`
keeps the no-partial-map guarantee while saving what's current; if a later
preview slice ever wrote junk geometry into the shared map, that would corrupt
co-editors and the canvas alike and needs preventing there, not compensating
here.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -133,25 +157,164 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
this.workflowActionService.disableWorkflowModification();
}
+ /**
+ * Size the name field to its text, the way the operator canvas does, so
what follows
+ * it starts at the same place in both views instead of after a fixed-width
box.
+ */
+ private adjustWorkflowNameWidth(): void {
+ const input =
this.host.nativeElement.querySelector<HTMLInputElement>("input.wf-name");
+ if (!input) {
+ return;
+ }
+ /* v8 ignore start -- font-metrics DOM measuring; jsdom has no layout */
+ const probe = document.createElement("span");
+ probe.style.visibility = "hidden";
+ probe.style.position = "absolute";
+ probe.style.whiteSpace = "pre";
+ probe.style.font = getComputedStyle(input).font;
+ probe.textContent = input.value || input.placeholder;
+ document.body.appendChild(probe);
+ input.style.width = `${Math.min(probe.offsetWidth + 20, 800)}px`;
+ document.body.removeChild(probe);
+ /* v8 ignore stop */
+ }
+
+ private refreshSavedState(): void {
+ const lastModified =
this.workflowActionService.getWorkflowMetadata()?.lastModifiedTime;
+ this.autoSaveState =
+ lastModified === undefined
+ ? ""
+ : "Saved at " +
+ (this.datePipe.transform(
+ lastModified,
+ "MM/dd/yyyy HH:mm:ss",
+ Intl.DateTimeFormat().resolvedOptions().timeZone,
+ "en"
+ ) ?? "");
+ }
+
+ /**
+ * Renaming here is the same edit as renaming on the operator canvas: commit
the name and
+ * save. The title bar itself -- the read-back (normalised) name and its
width -- is
+ * refreshed from the metadata subscription below, the single place this
page's own rename
+ * and a co-editor's both flow through.
+ */
+ public onRenameWorkflow(): void {
+ this.workflowActionService.setWorkflowName(this.workflowName);
+ this.save();
+ }
+
+ /**
+ * Keep the title bar in step with the workflow's metadata, exactly as the
operator canvas
+ * does: a rename or a save -- this page's own or a co-editor's -- refreshes
the name, its
+ * width, and the "Saved at ..." state from one place, so the two views
never drift apart.
+ */
+ private registerMetadataRefresh(): void {
+ this.workflowActionService
+ .workflowMetaDataChanged()
+ // The same 100ms the operator canvas debounces its title-bar refresh by.
+ .pipe(debounceTime(100), untilDestroyed(this))
+ .subscribe(() => {
+ this.workflowName =
this.workflowActionService.getWorkflowMetadata()?.name ?? "";
+ this.later(() => this.adjustWorkflowNameWidth(), 0);
+ this.refreshSavedState();
+ });
+ }
+
/**
* Switch to the operator canvas with a full page load, not a route. The two
views share
* root-level singletons (the graph, the Yjs shared model, the CU
connection); handing
* over in-process left the old state attached -- undraggable operators, a
ghost coeditor
* of yourself, broken runs. A fresh document is the reliable handover.
*/
public openRegularCanvas(): void {
+ this.save();
/* v8 ignore start -- full-document navigation; jsdom cannot navigate */
window.location.href = `${USER_WORKSPACE}/${this.wid}`;
/* v8 ignore stop */
}
+ /**
+ * Save the same way the operator canvas does. Both views edit one workflow,
so the
+ * form has to write through the same debounced persist -- otherwise an
author's
+ * setup, or a value someone filled in, would be gone on the next visit.
+ */
+ private registerAutoPersist(): void {
+ this.workflowActionService
+ .workflowChanged()
+ .pipe(debounceTime(SAVE_DEBOUNCE_TIME_IN_MS), untilDestroyed(this))
+ .subscribe(() => this.save());
+ }
+
+ /**
+ * Save the workflow this page opened, and only that one. The persist
endpoint creates a
+ * workflow when the payload has no id, so saving whatever the graph holds
would spawn
+ * stray "Untitled workflow" rows when the page is left before its workflow
loaded.
+ */
+ private save(): void {
+ if (!this.userService.isLogin() ||
!this.workflowPersistService.isWorkflowPersistEnabled()) {
+ return;
+ }
+ const workflow = this.workflowActionService.getWorkflow();
+ if (workflow.wid === undefined || workflow.wid !== this.wid) {
+ return;
+ }
+ const preserved: Workflow = {
+ ...workflow,
+ content: { ...workflow.content, operatorPositions:
this.positionsToSave(workflow.content) },
+ };
+ // On the way out the subscription must NOT be tied to this component:
ngOnDestroy
+ // calls save(), and untilDestroyed would tear the subscription down as
part of the
+ // very same destroy sequence, aborting the request that was the point of
the call.
+ const persist = this.workflowPersistService.persistWorkflow(preserved);
+ // The `destroyed` branch deliberately omits untilDestroyed (see above);
the persist call
+ // is a one-shot HTTP request that completes on its own, so it needs no
teardown operator.
+ // eslint-disable-next-line rxjs-angular/prefer-takeuntil
+ (this.destroyed ? persist : persist.pipe(untilDestroyed(this))).subscribe({
+ next: () => this.refreshSavedState(),
Review Comment:
The canvas's `persistWorkflow` feeds the response back —
`tap(updatedWorkflow => setWorkflowMetadata(updatedWorkflow))` in
menu.component — and that write is what advances `lastModifiedTime` (and the
normalised name); the metadata subscription then repaints. Here nothing updates
the metadata after a save, so this re-reads the load-time value and "Saved at
..." never moves past the moment the page opened. Mirroring the canvas's `tap`
(guarded by `destroyed` if needed) lets the subscription do the repaint and
makes this call redundant.
--
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]
