You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "yangzhang75 (via GitHub)" <gi...@apache.org> on 2026/09/09 02:00:30 UTC
[PR] feat(gui): author the Form View in place with an edit mode [texera]
yangzhang75 opened a new pull request, #8455:
URL: https://github.com/apache/texera/pull/8455
Part of the Form View stack (#8011), stacked on #8442; the review commit is the tip `e42687d85`.
Adds an in-place edit mode to the Form View. An author toggles into editing to rename or hide the exposed inputs and their sub-fields through an editable-label wrapper, reorder them, edit the instruction and help text, and choose which result cards to feature. A reader sees none of this: the inspect panel is `inert` and the graph is not modifiable.
Closes #8026
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(gui): author the Form View in place with an edit mode [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8455:
URL: https://github.com/apache/texera/pull/8455#discussion_r3991433708
##########
frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html:
##########
@@ -0,0 +1,61 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+-->
+
+<!-- Authoring: the label is the input, so what you type is what the reader reads. -->
+<div
+ class="lbl-row"
+ *ngIf="props.authoring">
+ <input
+ class="lbl-input"
+ [value]="props.authorName"
+ [placeholder]="props.schemaLabel"
+ (change)="onRename($event)"
Review Comment:
In authoring mode this textbox is only the label editor; it is not programmatically associated with the Formly control rendered by `fieldComponent`. Because `decorate()` also clears `props.label`, the actual scalar/sub-field control below has no accessible name. Preserve a programmatic label for the generated field id (for example, a visually hidden label that tracks `authorName || schemaLabel`) while keeping this input as the visible editor.
This issue also appears on line 52 of the same file.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -462,34 +536,93 @@ export class WorkflowFormComponent implements OnInit, OnDestroy {
}
/**
- * Show the workflow rather than edit it: the graph shape and its properties are read-only
- * on this page. A later PR's authoring mode makes properties editable with write access.
+ * Lock or unlock editing: the graph and its properties are read-only unless a writer is in edit
+ * mode, which is the only state that unlocks them (see toggleAuthoring).
*/
private applyEditability(): void {
- this.workflowActionService.disableWorkflowModification();
+ // Edit mode with write access is the only state that makes the operator properties (and the
+ // embedded canvas) modifiable here; every other state locks them, so a reader -- or a writer
+ // just viewing -- cannot change the workflow through this page.
+ if (this.authoring && this.canEdit) {
+ this.workflowActionService.enableWorkflowModification();
Review Comment:
Enabling the global modification flag also unlocks the embedded editor's context menu. `WorkflowEditorComponent.structureLocked` only gates paper/keyboard interactions (`workflow-editor.component.ts:127-140`), while the context menu shows cut, paste, disable, and delete solely from this global flag (`context-menu.component.html:33-79,126-151`). Consequently, Edit mode can reshape or delete operators from a preview that is still passed `structureLocked=true`. Please propagate the structure lock to the context menu (or split property-edit permission from structural-edit permission) before enabling the panel.
This issue also appears in the following locations of the same file:
- line 598
- line 861
- line 1130
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(gui): author the Form View in place with an edit mode [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 closed pull request #8455: feat(gui): author the Form View in place with an edit mode
URL: https://github.com/apache/texera/pull/8455
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(gui): author the Form View in place with an edit mode [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8455:
URL: https://github.com/apache/texera/pull/8455#discussion_r3991882814
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -462,34 +536,93 @@ export class WorkflowFormComponent implements OnInit, OnDestroy {
}
/**
- * Show the workflow rather than edit it: the graph shape and its properties are read-only
- * on this page. A later PR's authoring mode makes properties editable with write access.
+ * Lock or unlock editing: the graph and its properties are read-only unless a writer is in edit
+ * mode, which is the only state that unlocks them (see toggleAuthoring).
*/
private applyEditability(): void {
- this.workflowActionService.disableWorkflowModification();
+ // Edit mode with write access is the only state that makes the operator properties (and the
+ // embedded canvas) modifiable here; every other state locks them, so a reader -- or a writer
+ // just viewing -- cannot change the workflow through this page.
+ if (this.authoring && this.canEdit) {
+ this.workflowActionService.enableWorkflowModification();
Review Comment:
Right, and the editor's own `structureLocked` note said as much. Fixed in 871649877: `texera-context-menu` takes `[structureLocked]` from the editor and gates cut, paste, delete and disable/enable on modification AND no lock (`canModify`); copy, the result toggles, execute-to and export do not re-shape the graph and are unchanged. Menu-level tests for each command, plus an editor test that opens the right-click menu on a locked editor and checks the lock arrived.
##########
frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html:
##########
@@ -0,0 +1,61 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+-->
+
+<!-- Authoring: the label is the input, so what you type is what the reader reads. -->
+<div
+ class="lbl-row"
+ *ngIf="props.authoring">
+ <input
+ class="lbl-input"
+ [value]="props.authorName"
+ [placeholder]="props.schemaLabel"
+ (change)="onRename($event)"
Review Comment:
Fixed in 871649877: while authoring, a visually hidden `<label [for]=id>` that follows the current name (the author's, else the schema's) sits beside the name box, so the control keeps an accessible name; test added. Line 52 is the reader's label, which already carries `for`; it is omitted only when there is no text at all to give.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(gui): author the Form View in place with an edit mode [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on PR #8455:
URL: https://github.com/apache/texera/pull/8455#issuecomment-5628596195
/request-review: @mengw15
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(gui): author the Form View in place with an edit mode [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8455:
URL: https://github.com/apache/texera/pull/8455#issuecomment-5594634021
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- **Contributors with relevant context:** `@aglinxinyuan`
You can notify them by mentioning `@aglinxinyuan` in a comment.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(gui): author the Form View in place with an edit mode [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8455:
URL: https://github.com/apache/texera/pull/8455#discussion_r3971600915
##########
frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.ts:
##########
@@ -0,0 +1,75 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Component } from "@angular/core";
+import { NgIf } from "@angular/common";
+import { NzIconDirective } from "ng-zorro-antd/icon";
+import { FieldWrapper, FormlyFieldConfig } from "@ngx-formly/core";
+import { merge } from "lodash-es";
+
+/**
+ * Lets an author rename or hide one field of the form in place: the label itself becomes
+ * the input, so what they type is exactly what the reader sees, where they see it (the
+ * schema's own labels -- "File Key", "Alias" -- describe the operator, not the reader's
+ * task). Renders as a plain label for anyone not authoring.
+ */
+@Component({
+ selector: "texera-editable-label-wrapper",
+ templateUrl: "./editable-label-wrapper.component.html",
+ styleUrls: ["./editable-label-wrapper.component.scss"],
Review Comment:
This component declares `imports` but is missing `standalone: true`. In Angular, `imports` in component metadata is only valid for standalone components; as-is this will fail compilation. Add `standalone: true` (and keep `imports`) or remove `imports` and declare dependencies via an NgModule.
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.ts:
##########
@@ -205,6 +228,11 @@ export class PropertyEditorComponent implements OnInit, OnDestroy, OnChanges {
@HostListener("window:beforeunload")
ngOnDestroy(): void {
+ // The Form View's read-only copy (persistPlacement=false) must not persist geometry: it is not
+ // the docked canvas panel, so writing these keys would overwrite the real panel's saved size.
+ if (!this.persistPlacement) {
+ return;
+ }
localStorage.setItem("right-panel-width", String(this.width));
localStorage.setItem("right-panel-height", String(this.height));
Review Comment:
Avoid an early `return` in `ngOnDestroy()` to ensure any future teardown logic added later in this method still runs even when `persistPlacement` is false. Prefer guarding only the localStorage persistence block with `if (this.persistPlacement) { ... }` and letting the rest of `ngOnDestroy` execute normally.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.html:
##########
@@ -221,6 +361,136 @@ <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>
+
+ <!-- Reader / inspect: the property editor carries the `inert` attribute (blocks pointer AND
+ keyboard AND focus) and does not broadcast or write, so a step opens read-only.
+ Authoring: the panel goes live -- inert off, exposeChoosing on (tick boxes to pick what
+ the form exposes), and it broadcasts/edits like the canvas, since edit mode has enabled
+ workflow modification. 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"
+ [hidden]="!selectedOperatorId">
+ <texera-property-editor
+ [exposeChoosing]="authoring"
+ [persistPlacement]="false"
+ [broadcastEditing]="authoring"
+ [attr.inert]="authoring ? null : ''"></texera-property-editor>
+ </div>
+ </div>
+ </section>
+
+ <!-- Results, under the workflow that produced them. The section keeps its place before a run so
+ the answer has a visible destination. -->
+ <section class="results">
+ <div class="pc-section-head"><span class="label">Results</span></div>
+ <p
+ class="results-empty"
+ *ngIf="resultIdsToShow.length === 0">
+ {{ isRunning ? "Working…" : hasRunFinished ? "This run produced no results to show." : "Press Run and the
+ results appear here." }}
+ </p>
+ <!-- A card only for a step that actually produced a result. Whether a Python UDF
+ yields one cannot be told from the graph, so a step earns its card at runtime rather than
+ sitting on a permanent "No result yet." (a download/publish step never would). -->
+ <ng-container *ngFor="let id of resultIdsToShow">
+ <div class="card result">
+ <div class="result-head">
+ <span>{{ resultLabel(id) }}</span>
+ <!-- Zooming only means something once there is a picture to zoom. -->
+ <span
+ class="result-zoom"
+ *ngIf="vizHasContent(id)">
+ <button
+ (click)="zoomResult(id, -1)"
+ [disabled]="resultZoom(id) <= 0"
+ nz-tooltip="Smaller"
+ aria-label="Smaller">
+ <i
+ nz-icon
+ nzType="minus"
+ aria-hidden="true"></i>
+ </button>
+ <button
+ (click)="zoomResult(id, 1)"
+ [disabled]="resultZoom(id) >= 2"
+ nz-tooltip="Bigger"
+ aria-label="Bigger">
Review Comment:
The tooltip directive usage looks incorrect for ng-zorro: `nz-tooltip=\"Smaller\"` typically enables the directive but does not set the tooltip title (which is usually bound via `nzTooltipTitle`). As written, this may render an empty tooltip or none at all. Update to the supported API (e.g., enable `nz-tooltip` and set `nzTooltipTitle` accordingly).
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -371,16 +526,108 @@ export class WorkflowFormComponent implements OnInit, OnDestroy {
return ["INPUT", "TEXTAREA", "SELECT"].includes(active.tagName) || active.isContentEditable;
}
+ private operators(): OperatorPredicate[] {
+ return this.workflowActionService.getTexeraGraph().getAllOperators();
+ }
+
+ /**
+ * Drop exposed inputs whose operator was deleted: they can never be filled, and a re-added
+ * operator gets a fresh id so they could not reconnect. Guarded to edit mode and after load, so a
+ * reader never mutates the workflow and a not-yet-seeded mid-load graph never deletes a still-valid
+ * input. Only the operator-gone case, not a transiently missing property schema.
+ */
+ private pruneBrokenBindings(): void {
+ if (this.loading || !this.authoring) {
+ return;
+ }
+ const graph = this.workflowActionService.getTexeraGraph();
+ const configFields = this.formBindingService.getConfig().fields;
+ const alive = configFields.filter(p => graph.hasOperator(p.operatorID));
+ if (alive.length !== configFields.length) {
+ this.formBindingService.setFields(alive);
+ }
+ }
Review Comment:
`pruneBrokenBindings` mutates the form-binding config (`setFields`) based only on `authoring`, not on write access (`canEdit`). Since `toggleAuthoring()` can be called even when `canEdit` is false (and your tests do this), this method can still modify config state for a read-only viewer. Guard mutation with `this.canEdit` as well (or ensure `authoring` cannot be enabled without write access in the component method, not just the template).
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -371,16 +526,108 @@ export class WorkflowFormComponent implements OnInit, OnDestroy {
return ["INPUT", "TEXTAREA", "SELECT"].includes(active.tagName) || active.isContentEditable;
}
+ private operators(): OperatorPredicate[] {
+ return this.workflowActionService.getTexeraGraph().getAllOperators();
+ }
+
+ /**
+ * Drop exposed inputs whose operator was deleted: they can never be filled, and a re-added
+ * operator gets a fresh id so they could not reconnect. Guarded to edit mode and after load, so a
+ * reader never mutates the workflow and a not-yet-seeded mid-load graph never deletes a still-valid
+ * input. Only the operator-gone case, not a transiently missing property schema.
+ */
+ private pruneBrokenBindings(): void {
+ if (this.loading || !this.authoring) {
+ return;
+ }
+ const graph = this.workflowActionService.getTexeraGraph();
+ const configFields = this.formBindingService.getConfig().fields;
+ const alive = configFields.filter(p => graph.hasOperator(p.operatorID));
+ if (alive.length !== configFields.length) {
+ this.formBindingService.setFields(alive);
+ }
+ }
+
+ /**
+ * The author's picker for the extra results: every non-terminal step with view-result ("the eye")
+ * on the canvas is offered, plus any already-chosen step (so a pick never vanishes from its own
+ * picker). The terminal step is not offered -- its result always shows and is not the author's to
+ * toggle. The shown flag mirrors the saved resultOperatorIds.
+ */
+ private rebuildResultChoices(): void {
+ // The picker is only shown while authoring, so a reader does no per-operator work.
+ if (!this.authoring) {
+ this.resultChoices = [];
+ return;
+ }
+ const viewed = this.workflowActionService.getTexeraGraph().getOperatorsToViewResult();
+ const chosen = this.formBindingService.getConfig().resultOperatorIds;
+ // The terminal result always shows and is not the author's to toggle, so it is not offered here.
+ // The picker curates only the extra intermediate steps -- those given view-result (the eye) on the
+ // canvas. Reuse the one terminal rule (terminalOperatorIds) rather than a second copy. Already-chosen
+ // ids stay listed so the author can un-pick them.
+ const terminals = new Set(this.terminalOperatorIds());
+ this.resultChoices = this.operators()
+ .filter(op => !terminals.has(op.operatorID) && (viewed.has(op.operatorID) || chosen.includes(op.operatorID)))
+ .map(op => ({
+ operatorID: op.operatorID,
+ label: this.formBindingService.operatorLabel(op),
+ shown: chosen.includes(op.operatorID),
Review Comment:
`chosen.includes(op.operatorID)` is evaluated multiple times per operator, making this O(n*m) for n operators and m chosen ids. Convert `chosen` to a `Set` once (e.g., `const chosenSet = new Set(chosen)`) and use `chosenSet.has(...)` in both the filter and map.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.html:
##########
@@ -221,6 +361,136 @@ <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>
+
+ <!-- Reader / inspect: the property editor carries the `inert` attribute (blocks pointer AND
+ keyboard AND focus) and does not broadcast or write, so a step opens read-only.
+ Authoring: the panel goes live -- inert off, exposeChoosing on (tick boxes to pick what
+ the form exposes), and it broadcasts/edits like the canvas, since edit mode has enabled
+ workflow modification. 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"
+ [hidden]="!selectedOperatorId">
+ <texera-property-editor
+ [exposeChoosing]="authoring"
+ [persistPlacement]="false"
+ [broadcastEditing]="authoring"
+ [attr.inert]="authoring ? null : ''"></texera-property-editor>
+ </div>
+ </div>
+ </section>
+
+ <!-- Results, under the workflow that produced them. The section keeps its place before a run so
+ the answer has a visible destination. -->
+ <section class="results">
+ <div class="pc-section-head"><span class="label">Results</span></div>
+ <p
+ class="results-empty"
+ *ngIf="resultIdsToShow.length === 0">
+ {{ isRunning ? "Working…" : hasRunFinished ? "This run produced no results to show." : "Press Run and the
+ results appear here." }}
+ </p>
+ <!-- A card only for a step that actually produced a result. Whether a Python UDF
+ yields one cannot be told from the graph, so a step earns its card at runtime rather than
+ sitting on a permanent "No result yet." (a download/publish step never would). -->
+ <ng-container *ngFor="let id of resultIdsToShow">
+ <div class="card result">
+ <div class="result-head">
+ <span>{{ resultLabel(id) }}</span>
+ <!-- Zooming only means something once there is a picture to zoom. -->
+ <span
+ class="result-zoom"
+ *ngIf="vizHasContent(id)">
+ <button
+ (click)="zoomResult(id, -1)"
+ [disabled]="resultZoom(id) <= 0"
+ nz-tooltip="Smaller"
+ aria-label="Smaller">
Review Comment:
The tooltip directive usage looks incorrect for ng-zorro: `nz-tooltip=\"Smaller\"` typically enables the directive but does not set the tooltip title (which is usually bound via `nzTooltipTitle`). As written, this may render an empty tooltip or none at all. Update to the supported API (e.g., enable `nz-tooltip` and set `nzTooltipTitle` accordingly).
##########
frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html:
##########
@@ -0,0 +1,58 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+-->
+
+<!-- Authoring: the label is the input, so what you type is what the reader reads. -->
+<div
+ class="lbl-row"
+ *ngIf="props.authoring">
+ <input
+ class="lbl-input"
+ [value]="props.authorName"
+ [placeholder]="props.schemaLabel"
+ (change)="onRename($event)"
Review Comment:
Using the `(change)` event means renames only propagate on blur/commit, not as the author types. For in-place authoring this is often unintuitive and can make previewing/validation lag behind. Consider switching to `(input)` (or `(ngModelChange)` if you move to ngModel) so renames apply immediately.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -371,16 +526,108 @@ export class WorkflowFormComponent implements OnInit, OnDestroy {
return ["INPUT", "TEXTAREA", "SELECT"].includes(active.tagName) || active.isContentEditable;
}
+ private operators(): OperatorPredicate[] {
+ return this.workflowActionService.getTexeraGraph().getAllOperators();
+ }
+
+ /**
+ * Drop exposed inputs whose operator was deleted: they can never be filled, and a re-added
+ * operator gets a fresh id so they could not reconnect. Guarded to edit mode and after load, so a
+ * reader never mutates the workflow and a not-yet-seeded mid-load graph never deletes a still-valid
+ * input. Only the operator-gone case, not a transiently missing property schema.
+ */
+ private pruneBrokenBindings(): void {
+ if (this.loading || !this.authoring) {
+ return;
+ }
+ const graph = this.workflowActionService.getTexeraGraph();
+ const configFields = this.formBindingService.getConfig().fields;
+ const alive = configFields.filter(p => graph.hasOperator(p.operatorID));
+ if (alive.length !== configFields.length) {
+ this.formBindingService.setFields(alive);
+ }
+ }
+
+ /**
+ * The author's picker for the extra results: every non-terminal step with view-result ("the eye")
+ * on the canvas is offered, plus any already-chosen step (so a pick never vanishes from its own
+ * picker). The terminal step is not offered -- its result always shows and is not the author's to
+ * toggle. The shown flag mirrors the saved resultOperatorIds.
+ */
+ private rebuildResultChoices(): void {
+ // The picker is only shown while authoring, so a reader does no per-operator work.
+ if (!this.authoring) {
+ this.resultChoices = [];
+ return;
+ }
+ const viewed = this.workflowActionService.getTexeraGraph().getOperatorsToViewResult();
+ const chosen = this.formBindingService.getConfig().resultOperatorIds;
+ // The terminal result always shows and is not the author's to toggle, so it is not offered here.
+ // The picker curates only the extra intermediate steps -- those given view-result (the eye) on the
+ // canvas. Reuse the one terminal rule (terminalOperatorIds) rather than a second copy. Already-chosen
+ // ids stay listed so the author can un-pick them.
+ const terminals = new Set(this.terminalOperatorIds());
+ this.resultChoices = this.operators()
+ .filter(op => !terminals.has(op.operatorID) && (viewed.has(op.operatorID) || chosen.includes(op.operatorID)))
+ .map(op => ({
+ operatorID: op.operatorID,
+ label: this.formBindingService.operatorLabel(op),
+ shown: chosen.includes(op.operatorID),
Review Comment:
`chosen.includes(op.operatorID)` is evaluated multiple times per operator, making this O(n*m) for n operators and m chosen ids. Convert `chosen` to a `Set` once (e.g., `const chosenSet = new Set(chosen)`) and use `chosenSet.has(...)` in both the filter and map.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(gui): author the Form View in place with an edit mode [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on PR #8455:
URL: https://github.com/apache/texera/pull/8455#issuecomment-5638897605
Closing: at 2200 lines this was too much for one review, so it is split in two, both rebased onto current main (which now carries #8498 and #8499, so the duplicated rebuild-while-typing hunk is gone):
- #8516: the edit mode, with instruction and result authoring (closes #8026)
- #8517: authoring the inputs in place: rename, hide, reorder, remove (closes #8027, reused for this half), stacked on #8516
Together they are byte-for-byte the tree that was here at `01d0e2d35`, rebased, plus two strengthened tests; every fix from this PR's review threads is in one of the two. #8456 is now stacked on #8517.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(gui): author the Form View in place with an edit mode [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #8455:
URL: https://github.com/apache/texera/pull/8455#issuecomment-5594706114
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/8455?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `55.90551%` with `168 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 93.49%. Comparing base ([`a268c1c`](https://app.codecov.io/gh/apache/texera/commit/a268c1cc0d36ef9452fee54de6336483aefae17a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`e42687d`](https://app.codecov.io/gh/apache/texera/commit/e42687d85c1ade5aa1faeba68f5e04ac2ee73d3d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/texera/pull/8455?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...mponent/workflow-form/workflow-form.component.html](https://app.codecov.io/gh/apache/texera/pull/8455?src=pr&el=tree&filepath=frontend%2Fsrc%2Fapp%2Fworkspace%2Fcomponent%2Fworkflow-form%2Fworkflow-form.component.html&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZnJvbnRlbmQvc3JjL2FwcC93b3Jrc3BhY2UvY29tcG9uZW50L3dvcmtmbG93LWZvcm0vd29ya2Zsb3ctZm9ybS5jb21wb25lbnQuaHRtbA==) | 0.00% | [151 Missing :warning: ](https://app.codecov.io/gh/apache/texera/pull/8455?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...component/workflow-form/workflow-form.component.ts](https://app.codecov.io/gh/apache/texera/pull/8455?src=pr&el=tree&filepath=frontend%2Fsrc%2Fapp%2Fworkspace%2Fcomponent%2Fworkflow-form%2Fworkflow-form.component.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZnJvbnRlbmQvc3JjL2FwcC93b3Jrc3BhY2UvY29tcG9uZW50L3dvcmtmbG93LWZvcm0vd29ya2Zsb3ctZm9ybS5jb21wb25lbnQudHM=) | 95.39% | [1 Missing and 6 partials :warning: ](https://app.codecov.io/gh/apache/texera/pull/8455?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...abel-wrapper/editable-label-wrapper.component.html](https://app.codecov.io/gh/apache/texera/pull/8455?src=pr&el=tree&filepath=frontend%2Fsrc%2Fapp%2Fcommon%2Fformly%2Feditable-label-wrapper%2Feditable-label-wrapper.component.html&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZnJvbnRlbmQvc3JjL2FwcC9jb21tb24vZm9ybWx5L2VkaXRhYmxlLWxhYmVsLXdyYXBwZXIvZWRpdGFibGUtbGFiZWwtd3JhcHBlci5jb21wb25lbnQuaHRtbA==) | 71.42% | [2 Missing and 4 partials :warning: ](https://app.codecov.io/gh/apache/texera/pull/8455?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ponent/workflow-form/workflow-form.spec-harness.ts](https://app.codecov.io/gh/apache/texera/pull/8455?src=pr&el=tree&filepath=frontend%2Fsrc%2Fapp%2Fworkspace%2Fcomponent%2Fworkflow-form%2Fworkflow-form.spec-harness.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZnJvbnRlbmQvc3JjL2FwcC93b3Jrc3BhY2UvY29tcG9uZW50L3dvcmtmbG93LWZvcm0vd29ya2Zsb3ctZm9ybS5zcGVjLWhhcm5lc3MudHM=) | 90.00% | [2 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/texera/pull/8455?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...onent/property-editor/property-editor.component.ts](https://app.codecov.io/gh/apache/texera/pull/8455?src=pr&el=tree&filepath=frontend%2Fsrc%2Fapp%2Fworkspace%2Fcomponent%2Fproperty-editor%2Fproperty-editor.component.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZnJvbnRlbmQvc3JjL2FwcC93b3Jrc3BhY2UvY29tcG9uZW50L3Byb3BlcnR5LWVkaXRvci9wcm9wZXJ0eS1lZGl0b3IuY29tcG9uZW50LnRz) | 90.90% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/texera/pull/8455?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #8455 +/- ##
============================================
- Coverage 93.94% 93.49% -0.46%
Complexity 4824 4824
============================================
Files 1209 1211 +2
Lines 49618 50026 +408
Branches 6056 6118 +62
============================================
+ Hits 46615 46771 +156
- Misses 1522 1761 +239
- Partials 1481 1494 +13
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/8455/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/8455/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.00% <ø> (ø)` | | Carriedforward from [44bc1e3](https://app.codecov.io/gh/apache/texera/commit/44bc1e3719e96bac5c147868814ba55f699f35c6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/8455/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `99.32% <ø> (ø)` | | Carriedforward from [44bc1e3](https://app.codecov.io/gh/apache/texera/commit/44bc1e3719e96bac5c147868814ba55f699f35c6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/8455/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `89.91% <ø> (ø)` | | Carriedforward from [44bc1e3](https://app.codecov.io/gh/apache/texera/commit/44bc1e3719e96bac5c147868814ba55f699f35c6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/8455/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `75.05% <ø> (ø)` | | Carriedforward from [44bc1e3](https://app.codecov.io/gh/apache/texera/commit/44bc1e3719e96bac5c147868814ba55f699f35c6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/8455/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `87.12% <ø> (ø)` | | Carriedforward from [44bc1e3](https://app.codecov.io/gh/apache/texera/commit/44bc1e3719e96bac5c147868814ba55f699f35c6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/8455/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.65% <ø> (ø)` | | Carriedforward from [44bc1e3](https://app.codecov.io/gh/apache/texera/commit/44bc1e3719e96bac5c147868814ba55f699f35c6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/8455/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `95.71% <55.90%> (-1.06%)` | :arrow_down: | |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/8455/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.57% <ø> (ø)` | | Carriedforward from [44bc1e3](https://app.codecov.io/gh/apache/texera/commit/44bc1e3719e96bac5c147868814ba55f699f35c6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/8455/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `98.47% <ø> (ø)` | | Carriedforward from [44bc1e3](https://app.codecov.io/gh/apache/texera/commit/44bc1e3719e96bac5c147868814ba55f699f35c6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/8455/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.19% <ø> (ø)` | | Carriedforward from [44bc1e3](https://app.codecov.io/gh/apache/texera/commit/44bc1e3719e96bac5c147868814ba55f699f35c6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/8455?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(gui): author the Form View in place with an edit mode [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8455:
URL: https://github.com/apache/texera/pull/8455#discussion_r3984318387
##########
frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.ts:
##########
@@ -0,0 +1,75 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Component } from "@angular/core";
+import { NgIf } from "@angular/common";
+import { NzIconDirective } from "ng-zorro-antd/icon";
+import { FieldWrapper, FormlyFieldConfig } from "@ngx-formly/core";
+import { merge } from "lodash-es";
+
+/**
+ * Lets an author rename or hide one field of the form in place: the label itself becomes
+ * the input, so what they type is exactly what the reader sees, where they see it (the
+ * schema's own labels -- "File Key", "Alias" -- describe the operator, not the reader's
+ * task). Renders as a plain label for anyone not authoring.
+ */
+@Component({
+ selector: "texera-editable-label-wrapper",
+ templateUrl: "./editable-label-wrapper.component.html",
+ styleUrls: ["./editable-label-wrapper.component.scss"],
Review Comment:
Not needed on this codebase: Angular 21 (package.json pins 21.2.19) makes components standalone by default, so imports is valid without the flag, and the rest of the repo omits it the same way (e.g. DatasetSelectionModalComponent). It compiles and AOT builds as is.
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.ts:
##########
@@ -205,6 +228,11 @@ export class PropertyEditorComponent implements OnInit, OnDestroy, OnChanges {
@HostListener("window:beforeunload")
ngOnDestroy(): void {
+ // The Form View's read-only copy (persistPlacement=false) must not persist geometry: it is not
+ // the docked canvas panel, so writing these keys would overwrite the real panel's saved size.
+ if (!this.persistPlacement) {
+ return;
+ }
localStorage.setItem("right-panel-width", String(this.width));
localStorage.setItem("right-panel-height", String(this.height));
Review Comment:
Done, in #8442 where that code lives (this PR sits on it): the persistence block is now inside if (this.persistPlacement) and nothing returns early, so anything added to ngOnDestroy later runs for both mounts.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.html:
##########
@@ -221,6 +361,136 @@ <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>
+
+ <!-- Reader / inspect: the property editor carries the `inert` attribute (blocks pointer AND
+ keyboard AND focus) and does not broadcast or write, so a step opens read-only.
+ Authoring: the panel goes live -- inert off, exposeChoosing on (tick boxes to pick what
+ the form exposes), and it broadcasts/edits like the canvas, since edit mode has enabled
+ workflow modification. 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"
+ [hidden]="!selectedOperatorId">
+ <texera-property-editor
+ [exposeChoosing]="authoring"
+ [persistPlacement]="false"
+ [broadcastEditing]="authoring"
+ [attr.inert]="authoring ? null : ''"></texera-property-editor>
+ </div>
+ </div>
+ </section>
+
+ <!-- Results, under the workflow that produced them. The section keeps its place before a run so
+ the answer has a visible destination. -->
+ <section class="results">
+ <div class="pc-section-head"><span class="label">Results</span></div>
+ <p
+ class="results-empty"
+ *ngIf="resultIdsToShow.length === 0">
+ {{ isRunning ? "Working…" : hasRunFinished ? "This run produced no results to show." : "Press Run and the
+ results appear here." }}
+ </p>
+ <!-- A card only for a step that actually produced a result. Whether a Python UDF
+ yields one cannot be told from the graph, so a step earns its card at runtime rather than
+ sitting on a permanent "No result yet." (a download/publish step never would). -->
+ <ng-container *ngFor="let id of resultIdsToShow">
+ <div class="card result">
+ <div class="result-head">
+ <span>{{ resultLabel(id) }}</span>
+ <!-- Zooming only means something once there is a picture to zoom. -->
+ <span
+ class="result-zoom"
+ *ngIf="vizHasContent(id)">
+ <button
+ (click)="zoomResult(id, -1)"
+ [disabled]="resultZoom(id) <= 0"
+ nz-tooltip="Smaller"
+ aria-label="Smaller">
Review Comment:
This is a supported form: ng-zorro NzTooltipDirective declares the nz-tooltip attribute itself as an input (directiveTitle: ["nz-tooltip", ...] in ng-zorro-antd-tooltip.mjs), so nz-tooltip="Smaller" sets the title. The repo uses it everywhere, e.g. nz-tooltip="Customize Operator Name" on the property panel.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.html:
##########
@@ -221,6 +361,136 @@ <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>
+
+ <!-- Reader / inspect: the property editor carries the `inert` attribute (blocks pointer AND
+ keyboard AND focus) and does not broadcast or write, so a step opens read-only.
+ Authoring: the panel goes live -- inert off, exposeChoosing on (tick boxes to pick what
+ the form exposes), and it broadcasts/edits like the canvas, since edit mode has enabled
+ workflow modification. 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"
+ [hidden]="!selectedOperatorId">
+ <texera-property-editor
+ [exposeChoosing]="authoring"
+ [persistPlacement]="false"
+ [broadcastEditing]="authoring"
+ [attr.inert]="authoring ? null : ''"></texera-property-editor>
+ </div>
+ </div>
+ </section>
+
+ <!-- Results, under the workflow that produced them. The section keeps its place before a run so
+ the answer has a visible destination. -->
+ <section class="results">
+ <div class="pc-section-head"><span class="label">Results</span></div>
+ <p
+ class="results-empty"
+ *ngIf="resultIdsToShow.length === 0">
+ {{ isRunning ? "Working…" : hasRunFinished ? "This run produced no results to show." : "Press Run and the
+ results appear here." }}
+ </p>
+ <!-- A card only for a step that actually produced a result. Whether a Python UDF
+ yields one cannot be told from the graph, so a step earns its card at runtime rather than
+ sitting on a permanent "No result yet." (a download/publish step never would). -->
+ <ng-container *ngFor="let id of resultIdsToShow">
+ <div class="card result">
+ <div class="result-head">
+ <span>{{ resultLabel(id) }}</span>
+ <!-- Zooming only means something once there is a picture to zoom. -->
+ <span
+ class="result-zoom"
+ *ngIf="vizHasContent(id)">
+ <button
+ (click)="zoomResult(id, -1)"
+ [disabled]="resultZoom(id) <= 0"
+ nz-tooltip="Smaller"
+ aria-label="Smaller">
+ <i
+ nz-icon
+ nzType="minus"
+ aria-hidden="true"></i>
+ </button>
+ <button
+ (click)="zoomResult(id, 1)"
+ [disabled]="resultZoom(id) >= 2"
+ nz-tooltip="Bigger"
+ aria-label="Bigger">
Review Comment:
Same as the sibling thread: nz-tooltip="..." is the directive title input in ng-zorro (directiveTitle), not just an enabler, and it is the form used across the repo.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -371,16 +526,108 @@ export class WorkflowFormComponent implements OnInit, OnDestroy {
return ["INPUT", "TEXTAREA", "SELECT"].includes(active.tagName) || active.isContentEditable;
}
+ private operators(): OperatorPredicate[] {
+ return this.workflowActionService.getTexeraGraph().getAllOperators();
+ }
+
+ /**
+ * Drop exposed inputs whose operator was deleted: they can never be filled, and a re-added
+ * operator gets a fresh id so they could not reconnect. Guarded to edit mode and after load, so a
+ * reader never mutates the workflow and a not-yet-seeded mid-load graph never deletes a still-valid
+ * input. Only the operator-gone case, not a transiently missing property schema.
+ */
+ private pruneBrokenBindings(): void {
+ if (this.loading || !this.authoring) {
+ return;
+ }
+ const graph = this.workflowActionService.getTexeraGraph();
+ const configFields = this.formBindingService.getConfig().fields;
+ const alive = configFields.filter(p => graph.hasOperator(p.operatorID));
+ if (alive.length !== configFields.length) {
+ this.formBindingService.setFields(alive);
+ }
+ }
Review Comment:
Fixed, both halves. toggleAuthoring now refuses to enter edit mode without canEdit (leaving is always allowed), so the guard is at the method rather than only on the button; the test you noticed that entered edit mode as a reader now asserts it cannot. And pruneBrokenBindings is gone altogether (see the #8456 thread on it): it was deleting a broken binding before resolveFields could show it as the removable card this PR describes, so the only remaining config writes in edit mode are explicit author actions.
##########
frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html:
##########
@@ -0,0 +1,58 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+-->
+
+<!-- Authoring: the label is the input, so what you type is what the reader reads. -->
+<div
+ class="lbl-row"
+ *ngIf="props.authoring">
+ <input
+ class="lbl-input"
+ [value]="props.authorName"
+ [placeholder]="props.schemaLabel"
+ (change)="onRename($event)"
Review Comment:
Deliberate: the name commits on blur or Enter, the same way the workflow name input does (change on the canvas title). Writing on every keystroke would persist half-typed names into the shared config and rebuild the form under the cursor; there is no live preview elsewhere on the page that lags because of it.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -371,16 +526,108 @@ export class WorkflowFormComponent implements OnInit, OnDestroy {
return ["INPUT", "TEXTAREA", "SELECT"].includes(active.tagName) || active.isContentEditable;
}
+ private operators(): OperatorPredicate[] {
+ return this.workflowActionService.getTexeraGraph().getAllOperators();
+ }
+
+ /**
+ * Drop exposed inputs whose operator was deleted: they can never be filled, and a re-added
+ * operator gets a fresh id so they could not reconnect. Guarded to edit mode and after load, so a
+ * reader never mutates the workflow and a not-yet-seeded mid-load graph never deletes a still-valid
+ * input. Only the operator-gone case, not a transiently missing property schema.
+ */
+ private pruneBrokenBindings(): void {
+ if (this.loading || !this.authoring) {
+ return;
+ }
+ const graph = this.workflowActionService.getTexeraGraph();
+ const configFields = this.formBindingService.getConfig().fields;
+ const alive = configFields.filter(p => graph.hasOperator(p.operatorID));
+ if (alive.length !== configFields.length) {
+ this.formBindingService.setFields(alive);
+ }
+ }
+
+ /**
+ * The author's picker for the extra results: every non-terminal step with view-result ("the eye")
+ * on the canvas is offered, plus any already-chosen step (so a pick never vanishes from its own
+ * picker). The terminal step is not offered -- its result always shows and is not the author's to
+ * toggle. The shown flag mirrors the saved resultOperatorIds.
+ */
+ private rebuildResultChoices(): void {
+ // The picker is only shown while authoring, so a reader does no per-operator work.
+ if (!this.authoring) {
+ this.resultChoices = [];
+ return;
+ }
+ const viewed = this.workflowActionService.getTexeraGraph().getOperatorsToViewResult();
+ const chosen = this.formBindingService.getConfig().resultOperatorIds;
+ // The terminal result always shows and is not the author's to toggle, so it is not offered here.
+ // The picker curates only the extra intermediate steps -- those given view-result (the eye) on the
+ // canvas. Reuse the one terminal rule (terminalOperatorIds) rather than a second copy. Already-chosen
+ // ids stay listed so the author can un-pick them.
+ const terminals = new Set(this.terminalOperatorIds());
+ this.resultChoices = this.operators()
+ .filter(op => !terminals.has(op.operatorID) && (viewed.has(op.operatorID) || chosen.includes(op.operatorID)))
+ .map(op => ({
+ operatorID: op.operatorID,
+ label: this.formBindingService.operatorLabel(op),
+ shown: chosen.includes(op.operatorID),
Review Comment:
Done: chosen is a Set now, has() in both places.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -371,16 +526,108 @@ export class WorkflowFormComponent implements OnInit, OnDestroy {
return ["INPUT", "TEXTAREA", "SELECT"].includes(active.tagName) || active.isContentEditable;
}
+ private operators(): OperatorPredicate[] {
+ return this.workflowActionService.getTexeraGraph().getAllOperators();
+ }
+
+ /**
+ * Drop exposed inputs whose operator was deleted: they can never be filled, and a re-added
+ * operator gets a fresh id so they could not reconnect. Guarded to edit mode and after load, so a
+ * reader never mutates the workflow and a not-yet-seeded mid-load graph never deletes a still-valid
+ * input. Only the operator-gone case, not a transiently missing property schema.
+ */
+ private pruneBrokenBindings(): void {
+ if (this.loading || !this.authoring) {
+ return;
+ }
+ const graph = this.workflowActionService.getTexeraGraph();
+ const configFields = this.formBindingService.getConfig().fields;
+ const alive = configFields.filter(p => graph.hasOperator(p.operatorID));
+ if (alive.length !== configFields.length) {
+ this.formBindingService.setFields(alive);
+ }
+ }
+
+ /**
+ * The author's picker for the extra results: every non-terminal step with view-result ("the eye")
+ * on the canvas is offered, plus any already-chosen step (so a pick never vanishes from its own
+ * picker). The terminal step is not offered -- its result always shows and is not the author's to
+ * toggle. The shown flag mirrors the saved resultOperatorIds.
+ */
+ private rebuildResultChoices(): void {
+ // The picker is only shown while authoring, so a reader does no per-operator work.
+ if (!this.authoring) {
+ this.resultChoices = [];
+ return;
+ }
+ const viewed = this.workflowActionService.getTexeraGraph().getOperatorsToViewResult();
+ const chosen = this.formBindingService.getConfig().resultOperatorIds;
+ // The terminal result always shows and is not the author's to toggle, so it is not offered here.
+ // The picker curates only the extra intermediate steps -- those given view-result (the eye) on the
+ // canvas. Reuse the one terminal rule (terminalOperatorIds) rather than a second copy. Already-chosen
+ // ids stay listed so the author can un-pick them.
+ const terminals = new Set(this.terminalOperatorIds());
+ this.resultChoices = this.operators()
+ .filter(op => !terminals.has(op.operatorID) && (viewed.has(op.operatorID) || chosen.includes(op.operatorID)))
+ .map(op => ({
+ operatorID: op.operatorID,
+ label: this.formBindingService.operatorLabel(op),
+ shown: chosen.includes(op.operatorID),
Review Comment:
Done together with the sibling thread: one Set, has() in both places.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(gui): author the Form View in place with an edit mode [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8455:
URL: https://github.com/apache/texera/pull/8455#discussion_r3984377787
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.ts:
##########
@@ -205,6 +228,11 @@ export class PropertyEditorComponent implements OnInit, OnDestroy, OnChanges {
@HostListener("window:beforeunload")
ngOnDestroy(): void {
+ // The Form View's read-only copy (persistPlacement=false) must not persist geometry: it is not
+ // the docked canvas panel, so writing these keys would overwrite the real panel's saved size.
+ if (!this.persistPlacement) {
+ return;
+ }
localStorage.setItem("right-panel-width", String(this.width));
localStorage.setItem("right-panel-height", String(this.height));
Review Comment:
Correction: #8442 merged before that push landed, so the change is in this PR instead (same shape: the persistence block sits inside if (this.persistPlacement), no early return).
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org