Copilot commented on code in PR #8442:
URL: https://github.com/apache/texera/pull/8442#discussion_r3945679502
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.html:
##########
@@ -139,7 +219,105 @@
<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"
+ [attr.inert]="''"></texera-property-editor>
Review Comment:
`inert` prevents user interaction, but it does not make this component
lifecycle read-only. When the operator frame is created, `rerenderEditorForm()`
calls `setOperatorVersion` (`operator-property-edit-frame.component.ts:637`),
which writes a differing metadata version into the shared model and feeds
`workflowChanged()`/autosave. Inspecting an older operator can therefore modify
and persist the workflow. Add an inspect/read-only mode that skips this version
write and any other initialization mutations.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -260,10 +454,27 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
}
private readConfig(): void {
+ const config = this.formBindingService.getConfig();
this.parameters = this.formBindingService.resolveFields();
+ this.instructionTitle = config.instruction?.title ?? "";
+ this.instructionBody = config.instruction?.body ?? "";
+ this.refreshShownResults();
+ // A reader always sees the instruction as rendered markdown.
+ void this.renderInstruction();
this.buildForm();
}
+ /**
+ * Limit the shown results to the author's chosen operators that STILL have
view-result on in the
+ * canvas. This is a pure display filter: it reads the canvas's view-result
set and never writes
+ * it, so a normal canvas user's result-viewing is unaffected. A chosen
operator whose view-result
+ * was turned off (or that was deleted) simply drops out here rather than
rendering a stale card.
+ */
+ private refreshShownResults(): void {
+ const viewed =
this.workflowActionService.getTexeraGraph().getOperatorsToViewResult();
+ this.shownResultIds =
this.formBindingService.getConfig().resultOperatorIds.filter(id =>
viewed.has(id));
Review Comment:
This filter is not refreshed when a co-editor toggles an operator's
view-result flag. That action emits `getViewResultOperatorsChangedStream()`,
but this component only calls `refreshShownResults()` during config reads and
result updates; an already-produced result can remain visible (or hidden)
indefinitely when no new result arrives. Subscribe to the graph's view-result
change stream after loading the workflow and refresh/change-detect there.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -161,8 +235,128 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
return;
}
this.wid = wid;
+ // Give the result tables a realistic height to page against, so they show
a screenful of rows
+ // instead of one. (~7 rows; the card scrolls for the rest.)
+ this.panelResizeService.changePanelSize(900, 560);
+ // Highlighting is off by default; turning it on is what makes a click on
a step select it,
+ // which is how a reader opens that step's panel to inspect it (and,
later, an author to expose).
+ this.workflowActionService.setHighlightingEnabled(true);
this.load(wid);
+ // Selecting a step on the embedded (read-only) canvas opens its property
panel read-only. The
+ // canvas is not editable, but highlighting still works, so reuse it
rather than teach the editor
+ // a second click mode.
+ this.workflowActionService
+ .getJointGraphWrapper()
+ .getJointOperatorHighlightStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(ids => {
+ if (ids.length === 1) {
+ this.onOperatorClicked(ids[0]);
Review Comment:
The highlight event contains only the IDs newly highlighted, not the
complete selection (`JointGraphWrapper.highlightOperators` emits its local
`highlightedOperatorIDs`). Shift-clicking a second operator therefore emits a
one-item array and leaves this overlay open for that ID, while
`PropertyEditorComponent` sees two current highlights and clears its content,
producing an empty panel. Derive the selected operator from
`getCurrentHighlightedOperatorIDs()` and clear the selection unless the current
total is exactly one; apply the same synchronization on unhighlight so going
from two selections back to one reopens the remaining operator.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -161,8 +235,128 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
return;
}
this.wid = wid;
+ // Give the result tables a realistic height to page against, so they show
a screenful of rows
+ // instead of one. (~7 rows; the card scrolls for the rest.)
+ this.panelResizeService.changePanelSize(900, 560);
+ // Highlighting is off by default; turning it on is what makes a click on
a step select it,
+ // which is how a reader opens that step's panel to inspect it (and,
later, an author to expose).
+ this.workflowActionService.setHighlightingEnabled(true);
this.load(wid);
+ // Selecting a step on the embedded (read-only) canvas opens its property
panel read-only. The
+ // canvas is not editable, but highlighting still works, so reuse it
rather than teach the editor
+ // a second click mode.
+ this.workflowActionService
+ .getJointGraphWrapper()
+ .getJointOperatorHighlightStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(ids => {
+ if (ids.length === 1) {
+ this.onOperatorClicked(ids[0]);
+ }
+ // The property panel announces "currently editing this operator" to
everyone sharing the
+ // workflow. That is right on the operator canvas; here it made this
page's own session show
+ // up as a co-editor -- the user's own name in colour over an operator
on the other view.
+ // Nobody co-edits a graph from a form, so this page stays silent on
that channel.
+
this.workflowActionService.getTexeraGraph().updateSharedModelAwareness("currentlyEditing",
undefined);
Review Comment:
Clearing `currentlyEditing` here does not keep the Form View silent. The
always-mounted `PropertyEditorComponent` handles the same highlight immediately
afterward, creates `OperatorPropertyEditFrameComponent`, and that frame
unconditionally publishes the selected ID from `rerenderEditorForm()`
(`operator-property-edit-frame.component.ts:634`). The final awareness value is
therefore the operator ID, so co-editors still see this reader as editing.
Propagate an inspect/read-only flag into the frame and suppress its awareness
publication in that mode.
--
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]
