yangzhang75 commented on code in PR #8442:
URL: https://github.com/apache/texera/pull/8442#discussion_r3945966495
##########
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:
Fixed. You are right that inert does not stop the lifecycle write:
rerenderEditorForm calls setOperatorVersion -> changeOperatorVersion, which
writes the version into the Yjs shared model. It is now gated behind
broadcastEditing (which the Form View sets false), so a read-only inspect
neither broadcasts editing nor syncs/writes the operator version. Added a frame
test asserting setOperatorVersion is not called when broadcastEditing is false.
##########
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:
Fixed: the handler now reads getCurrentHighlightedOperatorIDs() (the full
selection) instead of the emitted delta, matching the property panel's own
logic -- exactly one highlighted step opens the panel, and a shift-click
multi-select closes it. Added a test that models the delta emit plus the
full-set accessor.
##########
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:
Fixed (anchored to an older commit): the racy clear here is gone. The panel
is mounted with broadcastEditing=false, which suppresses the broadcast at the
frame (the only writer), so it cannot be re-set after this handler runs.
Covered by a frame test.
##########
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:
Fixed in the results PR #8441 (its diff shows cumulatively here): the
component subscribes to getViewResultOperatorsChangedStream() so a view-result
toggle refreshes the shown set even with no result update.
--
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]
