Copilot commented on code in PR #8516:
URL: https://github.com/apache/texera/pull/8516#discussion_r3995153730
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -983,6 +1137,78 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
this.instructionOpen = !this.instructionOpen;
}
+ //
---------------------------------------------------------------------------
+ // Author mode: editing the form in place (write access only). Graph edits
go through the same
+ // shared graph the operator canvas edits; form-binding edits go through the
form-binding config
+ // (local until #8351 shares it). Each edit then re-reads the config.
+ //
---------------------------------------------------------------------------
+
+ public toggleAuthoring(): void {
+ // Entering edit mode needs write access. The Edit button is only rendered
for a writer, but the
+ // guard belongs here, at the method, so no other caller can put a reader
into a mode whose every
+ // action writes the shared config. Leaving edit mode is always allowed.
+ if (!this.authoring && !this.canEdit) {
+ return;
+ }
+ this.authoring = !this.authoring;
+ if (this.authoring) {
+ // An author picks fields off the workflow, so show it.
+ this.showWorkflow();
+ // In edit mode the picker sets the default for everyone, so the
author's own view of the
+ // results (if they toggled any as a viewer) gives way to that default.
+ this.viewerResultIds = undefined;
+ } else {
+ this.workflowOpen = false;
+ }
Review Comment:
Leaving authoring keeps the selected operator mounted, but changing
`actsAsEditor` to false only remounts the property frame; neither that remount
nor the frame's teardown clears the `currentlyEditing` awareness value it
published in editor mode (`property-editor.component.ts:152-156`,
`operator-property-edit-frame.component.ts:649-653,619-622`). Co-editors can
therefore keep seeing this Form View session as editing the operator after
Done. Clear the awareness entry when leaving authoring (or close/unhighlight
the selected panel).
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.html:
##########
@@ -237,32 +343,36 @@ <h2>{{ instructionTitle || "How to use this" }}</h2>
aria-hidden="true"></i>
</button>
- <!-- A reader can open a step to view its settings, read-only.
Read-only is enforced twice,
- at different layers, because neither layer alone is enough:
- [actsAsEditor]="false" stops every write the panel can make
(co-editor
- awareness, operator version, operator properties) -- that is the
one that matters, since
- merely opening a step makes ajv fill in schema defaults and emit
a change; and `inert`
- stops the interaction, blocking pointer, keyboard and focus.
Disabling the form is not a
- substitute for `inert`: the preset save/apply/delete buttons, the
array add/remove
- buttons and the drag-to-reorder handles are plain controls that a
disabled FormGroup
- does not reach. Turning the panel live for choosing what to
expose is the authoring PR.
- The panel is the scroll container and takes the focus itself
(tabindex, aria-label), so
- a keyboard reader can still scroll a long panel that `inert`
content cannot hold focus in.
+ <!-- Reader / inspect: read-only is enforced twice, at different
layers, because neither
+ layer alone is enough. [actsAsEditor]="false" stops every write
the panel can make
+ (co-editor awareness, operator version, operator properties) --
that is the one that
+ matters, since merely opening a step makes ajv fill in schema
defaults and emit a
+ change; and `inert` stops the interaction, blocking pointer,
keyboard and focus.
+ Disabling the form is not a substitute for `inert`: the preset
save/apply/delete
+ buttons, the array add/remove buttons and the drag-to-reorder
handles are plain
+ controls that a disabled FormGroup does not reach. Since `inert`
content cannot hold
+ focus, the panel takes the tab stop itself so a keyboard reader
can still scroll a long
+ panel.
+ Authoring: the panel goes live -- inert off, acting as an editor,
exposeChoosing on
+ (tick boxes to pick what the form exposes), so it edits like the
canvas, which edit mode
+ has already enabled workflow modification for. The panel's own
tab stop goes away with
+ inert, so it does not sit in front of the form it now contains.
persistPlacement stays
+ off either way (this is not the docked canvas panel).
[hidden], not *ngIf: the property editor shows its operator by
REACTING to the highlight
stream (no initial pull), so it must already be mounted and
subscribed when the click
highlights a step. Mounting it on selection (*ngIf) subscribes
too late, misses that
emission, and the panel opens empty. So keep it mounted and just
hide it. -->
<div
class="panel"
role="group"
- tabindex="0"
- aria-label="Step settings, read-only"
+ [attr.tabindex]="authoring ? null : 0"
+ [attr.aria-label]="authoring ? 'Step settings' : 'Step settings,
read-only'"
[hidden]="!selectedOperatorId">
<texera-property-editor
- [exposeChoosing]="false"
+ [exposeChoosing]="authoring"
[persistPlacement]="false"
- [actsAsEditor]="false"
- [attr.inert]="''"></texera-property-editor>
+ [actsAsEditor]="authoring"
Review Comment:
Entering Edit during an active run still remounts a selected property frame
with `actsAsEditor=true`, even though `mayUnlock()` keeps the root lock off.
That frame does not consult the lock before editor-only writes: it synchronizes
the operator version on mount and its AJV-default form event writes whenever
`actsAsEditor` is true (`operator-property-edit-frame.component.ts:649-660,
715-727, 813-824`). A selected step can therefore mutate the running workflow
merely by entering Edit. Gate editor mode on the execution state as well.
--
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]
