yangzhang75 commented on code in PR #8581:
URL: https://github.com/apache/texera/pull/8581#discussion_r4075701539
##########
frontend/src/app/workspace/component/menu/menu.component.ts:
##########
@@ -711,9 +711,10 @@ export class MenuComponent implements OnInit, OnDestroy {
}
/**
- * Open the Form View -- a full page load, not a route: the two views share
root-level
- * singletons (graph, Yjs shared model), and routing left the old
collaboration client
- * alive (you appeared as your own coeditor). A fresh document is the clean
handover.
+ * Open the Form View. A route, not a page load: the two views are views of
one open workflow,
Review Comment:
Taken as you describe it: `openFormViewPage` takes the navigation's promise
and lowers `handingOverToFormView` when it resolves `false` or rejects; on
success the component is gone, flag and all. You are right that it is a loose
end of the old assumption rather than something reachable on purpose -- with
the page discarded the flag went with it, and a route leaves the page in place.
Two tests, one per branch; deletion-checked by discarding the result again.
4160529ad
##########
frontend/src/app/workspace/component/workspace.component.ts:
##########
@@ -197,11 +210,19 @@ export class WorkspaceComponent implements AfterViewInit,
OnInit, OnDestroy {
ngOnDestroy() {
this.persistBeforeLeaving();
this.codeEditorViewRef.clear();
- this.workflowActionService.clearWorkflow();
// Tear down the connection and all websocket-derived session state so a
- // re-entered workflow starts clean instead of reusing the previous one.
- this.computingUnitStatusService.disconnect();
- this.resetWorkflowSessionState();
+ // re-entered workflow starts clean instead of reusing the previous one --
unless the Form
Review Comment:
All eight rewritten, and thank you for separating the two kinds. The two
`beforeunload` docs now name what still fires them -- a refresh, a closed tab,
a URL typed over this one -- and say the switch used to be one and routes now,
so nobody reads the stated case, finds it impossible, and deletes the handler.
The six save-ordering comments state the reason that survives the change: a
save that fails should keep the writer on the view they edited in with the
error in front of them, and an edit made during the save should be stored here
rather than left to an autosave firing under the other view -- not a request
aborted by an unload, which no longer happens.
This was a miss of my own checklist rather than of the audit: the audit
covered behaviour, and a grep for the old wording is a separate step I did not
run. It is run now, and the remaining hits are about page reloads that still
are one (a refresh) or historical ("while every mount followed a page load").
4160529ad
--
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]
