Copilot commented on code in PR #8456:
URL: https://github.com/apache/texera/pull/8456#discussion_r3998675426
##########
frontend/src/app/workspace/component/menu/menu.component.ts:
##########
@@ -617,15 +628,99 @@ export class MenuComponent implements OnInit, OnDestroy {
}
public onClickExportWorkflow(): void {
- const workflowContent: WorkflowContent =
this.workflowActionService.getWorkflowContent();
- const workflowContentJson = JSON.stringify(workflowContent, null, 2);
+ // The same shape the dashboard download produces (see exportedWorkflow):
the content plus the
+ // landing view as a sibling key, so a file exported here uploads as a
form-default workflow too.
+ const exported = exportedWorkflow(
+ this.workflowActionService.getWorkflowContent(),
+ this.workflowActionService.getWorkflowMetadata().defaultView
+ );
+ const workflowContentJson = JSON.stringify(exported, null, 2);
const fileName = this.currentWorkflowName + ".json";
// Through the injectable wrapper (as the dashboard downloads already do),
so a spec stubs it
// with TestBed instead of module-mocking the CommonJS file-saver package,
which the unit-test
// builder cannot hoist reliably.
this.fileSaverService.saveAs(new Blob([workflowContentJson], { type:
"text/plain;charset=utf-8" }), fileName);
}
+ /**
+ * 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.
+ */
+ public onClickOpenFormView(): void {
+ const wid = this.workflowActionService.getWorkflowMetadata().wid;
+ if (wid === undefined || this.handingOverToFormView) {
+ return;
+ }
+ // A reader has nothing to save, and every save of theirs is a guaranteed
403 that would keep
+ // them here with an error: straight over, as the form's own switch does
for a reader.
+ if (!this.writeAccess) {
+ this.openFormViewPage(wid);
+ return;
+ }
+ // Save first, and hand over only once the save has completed. The
full-page load that
+ // follows unloads this document, and a request still in flight at that
moment is aborted, so
+ // navigating right after firing the save could lose the very edit the
switch is meant to carry
+ // across; the workspace's beforeunload save runs into the same unload and
is no safety net. A
+ // save that fails keeps the user here with the error shown, rather than
leaving with changes
+ // that were never stored. The form's own switch (openRegularCanvas) does
the same.
+ //
+ // Two more things the hand-over must not lose. An autosave already in
flight when the switch
+ // is clicked: WorkflowPersistService sends saves one at a time and in
order, so ours lands after
+ // it and completes after it. And an edit made while our save is out (the
page stays editable
+ // until the load): workflowChanged marks a graph edit, persistWorkflow a
rename or a
+ // description edit (those save through the menu itself, not the
autosave), and the drain below
+ // saves once more before handing over rather than letting the full-page
load abort that edit's
+ // own save.
+ this.handingOverToFormView = true;
+ this.isSaving = true;
+ this.saveThenOpenFormView(wid);
+ }
+
+ private saveThenOpenFormView(wid: number): void {
+ // The snapshot below carries everything reported up to now.
+ this.editedSinceSwitchSnapshot = false;
+ this.workflowPersistService
+ .persistWorkflow(this.workflowActionService.getWorkflow())
+ .pipe(untilDestroyed(this))
+ .subscribe({
+ next: (updatedWorkflow: Workflow) => {
+ // An edit since the snapshot makes this response stale: applying it
would put the old
+ // name back just before the save below re-reads the workflow. That
save's own response
+ // is the one applied.
+ if (!this.editedSinceSwitchSnapshot) {
+ this.workflowActionService.setWorkflowMetadata(updatedWorkflow);
Review Comment:
This stale-response guard only covers the switch's own save. An older
autosave or menu save still applies its response unconditionally
(`workspace.component.ts:217-222` and `menu.component.ts:778-780`) and can
overwrite a rename made while the hand-over is pending. The subsequent resave
then snapshots that reverted name and, because it is queued last, persists it
before navigation. Preserve locally changed metadata for every queued response
(or use revisioned snapshots centrally), not just this callback.
##########
frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.html:
##########
@@ -36,12 +36,31 @@
[(ngModel)]="entry.checked"
(ngModelChange)="onCheckboxChange(entry)"></label>
</div>
- <!-- Cover-image controls -->
+ <!-- Cover controls: the owner's image controls, and the default-view
toggle for anyone with
+ write access (the same toggle the list row offers, so switching to
the card view does not
+ lose it). -->
<div
class="card-image-controls"
- *ngIf="canEditCover"
+ *ngIf="hasCoverControls"
(click)="$event.stopPropagation()">
+ <!-- A toggle button: constant name, state in aria-pressed (a name that
changed with the state
+ would announce the opposite of what the state says); the title
spells out what a click does. -->
<button
+ *ngIf="canToggleDefaultView"
+ nz-button
+ nzType="text"
+ class="image-control-btn default-view-toggle"
Review Comment:
The containing `.card-image-controls` has `opacity: 0` and becomes visible
only on mouse hover (`card-item.component.scss:71-85`). The button remains
tabbable, but keyboard users focus an invisible control because focus does not
reveal the container. Add a `:focus-within` reveal rule (and a visible focus
style) so the toggle is operable without a pointer.
##########
frontend/src/app/dashboard/component/user/list-item/list-item.component.html:
##########
@@ -194,6 +195,26 @@
nz-icon
nzType="eye"></i>
</button>
+ <!-- Setting the default view is a write on the workflow row (the endpoint
requires WRITE), so a
+ read-only collaborator is not offered a control that could only fail
(the handler checks
+ the same rule, canToggleDefaultView). -->
+ <!-- A toggle button: the name stays constant and aria-pressed carries the
state (a name that
+ changed with the state would announce the opposite of what the state
says); the title spells
+ out what a click does. -->
+ <button
+ *ngIf="canToggleDefaultView"
+ nz-button
+ nzType="text"
+ class="default-view-toggle"
Review Comment:
This new toggle is inside `.button-group`, which is `display: none` until
`.list-item-card:hover` (`list-item.component.scss:99-124`). Keyboard users
cannot hover, and `display: none` removes the button from tab order, so they
cannot change the default view. Reveal the action group on `:focus-within` (and
provide a focusable trigger) or keep this toggle keyboard-visible.
##########
frontend/src/app/workspace/component/menu/menu.component.html:
##########
@@ -75,6 +75,30 @@
<span *ngIf="!displayParticularWorkflowVersion"> {{autoSaveState}}
</span>
</div>
+ <!-- One workflow, two ways of working on it: the Canvas and the Form
View. The
+ same control sits in the same place on both, so switching never
means hunting
+ for a different affordance; the pressed segment is the view you are
looking at.
+ Every workflow offers both views, so this shows wherever the
feature flag is on. -->
+ <div
+ class="view-switch"
+ *ngIf="this.config.env.formViewEnabled &&
!displayParticularWorkflowVersion">
Review Comment:
The checked-in production flag is still `false`
(`common/config/src/main/resources/gui.conf:89`), so every entry point added
here remains hidden and direct Form View links redirect to canvas. That
conflicts with the PR title and closing issue #8028, both of which require
turning the feature on. Either flip the default in this PR or stop
claiming/closing the rollout item.
--
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]
