yangzhang75 commented on code in PR #8442:
URL: https://github.com/apache/texera/pull/8442#discussion_r3982955786
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.html:
##########
@@ -221,6 +221,40 @@ <h2>{{ instructionTitle || "How to use this" }}</h2>
<texera-mini-map
*ngIf="workflowEverOpened"
class="box"></texera-mini-map>
+
+ <!-- Sibling of the panel, not nested inside it: the property editor
opens its own stacking
+ context, and a button inside that context paints under its
content -- visible but
+ unclickable. As a sibling the close button is simply above. -->
+ <button
+ class="panel-close"
+ *ngIf="selectedOperatorId"
+ type="button"
+ aria-label="Close step details"
+ (click)="closeOperatorPanel()">
+ <i
+ nz-icon
+ nzType="close"
+ aria-hidden="true"></i>
+ </button>
+
+ <!-- A reader can open a step to view its settings, read-only. The
panel itself is the
+ scroll container (so a long panel can still be read); the
property editor inside carries
+ the `inert` attribute, which blocks pointer AND keyboard AND
focus -- so nothing in it
+ can be edited or tabbed into. The graph is modification-disabled
too. Turning the panel
+ live for choosing what to expose is the authoring PR.
+ [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"
+ [hidden]="!selectedOperatorId">
+ <texera-property-editor
+ [exposeChoosing]="false"
+ [persistPlacement]="false"
+ [broadcastEditing]="false"
+ [attr.inert]="''"></texera-property-editor>
Review Comment:
Half taken, half not, and I want to be explicit about which. I fixed the
keyboard and announcement part: the panel itself now carries the tab stop plus
role=group and an aria-label, so a keyboard reader can scroll a long panel and
hears what the region is, which inert content cannot provide on its own. I am
keeping inert rather than swapping to a disabled presentation, because
disabling the form does not reach the controls that matter here. Three families
inside the panel are plain buttons with plain click handlers, untouched by a
disabled FormGroup: the preset save/apply/delete buttons in preset-wrapper, the
add/remove buttons in array.type.ts, and the cdkDrag reorder handles in
repeat-dnd. Applying a preset writes operator properties outright. So dropping
inert would turn a read-only panel into one a reader can visibly mutate, which
is a worse regression than the accessibility cost it buys back. The lifecycle
side of read-only is now enforced separately by actsAsEditor, so inert
is only the interaction layer, not the guarantee. If we want the settings
fully readable by assistive tech, the right fix is to make those three
components honour the disabled state and then drop inert; that is a change to
shared canvas components and I would rather do it as its own PR than fold it in
here. Happy to file it.
##########
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts:
##########
@@ -178,6 +178,11 @@ export class OperatorPropertyEditFrameComponent implements
OnInit, OnChanges, On
/** True while an author is choosing which properties appear on the Form
View; adds a tick
* box beside each. Off, the property editor is unchanged. */
@Input() exposeChoosing = false;
+ /** Whether opening an operator may write to the shared workflow: the
"currently editing" co-editor
+ * broadcast AND the operator-version sync (both are shared-model writes).
True on the operator
+ * canvas; the Form View sets it false to inspect a step read-only, so
opening one neither shows
+ * the reader as a co-editor nor mutates the operator's version. */
+ @Input() broadcastEditing = true;
Review Comment:
Agreed, and taken as named: renamed to actsAsEditor. It now decides four
things rather than one, so the narrow name would have aged badly exactly the
way you describe. Gating in this PR: the currentlyEditing presence broadcast,
the operator-version sync, the operator properties (both the form-change sink
and the UDF ui-parameter sync), and interactivity itself, which
setInteractivity clamps so a finished run or the runtime unlock cannot reopen
the form. The property writes are gated at their single sink rather than at
each caller, so a future write path has to pass through the check. The doc
comment on the input enumerates the four.
--
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]
