yangzhang75 commented on code in PR #8516:
URL: https://github.com/apache/texera/pull/8516#discussion_r3995284489
##########
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:
Confirmed: the frame syncs the version on mount and writes ajv defaults
whenever actsAsEditor is true, without consulting the lock. c66787a73 binds the
panel's actsAsEditor / exposeChoosing / inert to the lock's own rule
(panelLive: edit mode, write access, no run in flight), so a step selected
mid-run mounts read-only even in edit mode and turns live when the run ends.
Rendered spec covers both states; binding it to authoring alone turns it red.
The suppressed hand-over note is addressed too: navigation now waits for the
queue to drain (a save queued behind the switch's, and its failure keeping the
page), with specs for both.
##########
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:
Right, the viewer remount clears nothing and the property editor clears
currentlyEditing only while it acts as an editor. c66787a73 makes Done dismiss
the step panel (closeOperatorPanel, through the action service) before
authoring flips off, so the clear runs while the frame is still an editor. Spec
asserts the dismissal happens with authoring still true; deleting it turns the
spec red.
--
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]
