mengw15 commented on code in PR #8581:
URL: https://github.com/apache/texera/pull/8581#discussion_r4066701491
##########
frontend/src/app/workspace/component/workflow-editor/mini-map/mini-map.component.ts:
##########
@@ -88,8 +95,26 @@ export class MiniMapComponent implements AfterViewInit,
OnDestroy {
this.panelService.resetPanelStream.pipe(untilDestroyed(this)).subscribe(()
=> (this.hidden = false));
}
+ /**
+ * The browser is leaving this document: remember whether the mini-map was
hidden, and destroy
+ * nothing. The document may be kept in the back/forward cache and restored
with its JavaScript
+ * state exactly as it was left, re-running nothing, so a paper disposed
here would stay disposed
+ * on a page that looks live (the same reason the workspace stopped tearing
down here, #8599).
+ */
@HostListener("window:beforeunload")
+ onBeforeUnload(): void {
+ this.rememberVisibility();
+ }
+
ngOnDestroy(): void {
Review Comment:
`getMainJointPaperAttachedStream()` is a `ReplaySubject(1)`
(`joint-graph-wrapper.ts:105`) and the subscriber above registers three
`mainPaper.on(...)` with no matching `off`. This method removes only
`ownPaper`, so those three stay on whichever main paper the component last
received.
That is harmless while the main paper dies with its own editor. It stops
being harmless during the overlap this PR documents: #8606 measured two
`#workflow-editor` in the document, with the departing view's DOM still
attached when the arriving view runs `ngAfterViewInit`. Angular runs
`ngOnDestroy` before detaching a view's DOM, so a departing view whose DOM is
still there has not been destroyed yet and is still subscribed — it receives
the arriving paper, registers on it, and is destroyed a moment later. The
arriving paper outlives it and goes on calling `updateNavigator` on a component
that is gone, once more per switch. Nothing visible breaks: the writes land on
its own detached `#mini-map-navigator`. Same family as #8582.
Binding the three callbacks once and removing them from `this.paper` here
would close it.
##########
frontend/src/app/workspace/component/menu/menu.component.ts:
##########
@@ -197,17 +197,14 @@ export class MenuComponent implements OnInit, OnDestroy {
private jupyterPanelService: JupyterPanelService,
private fileSaverService: FileSaverService
) {
- workflowWebsocketService
- .subscribeToEvent("ExecutionDurationUpdateEvent")
- .pipe(
- tap(event => (this.executionDuration = event.duration)),
- // restart the 1s timer on each event, only while running
- switchMap(event => (event.isRunning ? timer(1000, 1000) : EMPTY)),
- untilDestroyed(this)
- )
- .subscribe(() => {
- this.executionDuration += 1000;
- });
+ // From the service, not from the engine's event directly: that event
arrives twice in a whole
+ // run, so a timer hung off it never started for a menu that mounted in
between -- which is what
+ // a routed switch between the canvas and the Form View makes. The service
anchors the clock and
+ // ticks it, and replays the current value to whoever subscribes.
+ executeWorkflowService
+ .getExecutionDurationStream()
Review Comment:
Now unused, orphaned by this change: `switchMap` (line 34), `EMPTY` and
`timer` (line 42). `workflow-form.component.ts` has `switchMap` and `timer` in
the same state. Neither the eslint config (no `no-unused-vars`) nor `tsconfig`
(no `noUnusedLocals`) flags them, so CI stays green either way.
--
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]
