yangzhang75 commented on code in PR #8581:
URL: https://github.com/apache/texera/pull/8581#discussion_r4065231837
##########
frontend/src/app/workspace/component/workflow-editor/mini-map/mini-map.component.ts:
##########
@@ -90,6 +97,10 @@ export class MiniMapComponent implements AfterViewInit,
OnDestroy {
@HostListener("window:beforeunload")
ngOnDestroy(): void {
+ // Bound to the root-provided joint graph, which outlives this component:
an undisposed paper
+ // goes on listening to that graph from a detached node, and once the
switch between a
+ // workflow's two views routes, one is left behind on every switch (issue
#8582).
+ this.ownPaper?.remove();
localStorage.setItem("mini-map", JSON.stringify(this.hidden));
}
Review Comment:
Fixed in b954174, which landed just before this thread. `beforeunload` is
its own handler and only remembers whether the mini-map was hidden;
`paper.remove()` moved to `ngOnDestroy`, which runs when the component is
genuinely destroyed. The restore-oriented assertion you asked for is there too:
dispatching a real `beforeunload` leaves `remove()` uncalled and still writes
the visibility flag, and it is deletion-checked by moving the disposal back
into the unload path. Same reasoning as #8599, on the page next door.
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -255,7 +263,17 @@ export class WorkflowEditorComponent implements OnInit,
AfterViewInit, OnDestroy
ngOnDestroy(): void {
this.paperResizeObserver?.disconnect();
- document.removeEventListener("keydown",
this._handleKeyboardAction.bind(this));
+ // The paper is bound to the joint graph, which is root-provided and
outlives this component,
+ // so an undisposed one goes on listening to that graph from a DOM node no
longer on the page.
+ // Harmless while every mount followed a page load; the switch between a
workflow's two views
+ // routes now, so a mount happens on every switch and the papers pile up.
Two live papers on
+ // one model both answer pointer events, and whichever answers decides
whether an operator can
+ // be dragged -- measured: an operator was undraggable after two
round-trips (issue #8582).
Review Comment:
Corrected, and thank you -- you are right that the thread is the wrong place
for a retraction that belongs in the line itself. The comment claimed both the
mechanism and a measurement as settled, and the measurement was an artefact:
the test read "the first .joint-element", which stops being the same element
once one has been dragged or selected.
It now says only that the papers pile up once remounting is the normal
switch, and that whether the undraggable operators in #8580 follow from that is
not settled, with #8582 tracking it. The PR description carried the same
overclaim, and still said the disposal was deferred when it is now in this PR;
both corrected. e7c2cbf
--
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]
