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/01 23:03:24 UTC
[PR] feat(property-editor): choose which properties the Form View exposes [texera]
yangzhang75 opened a new pull request, #8318:
URL: https://github.com/apache/texera/pull/8318
### What changes were proposed in this PR?
Part of the Form View feature stack (parent issue #8011). It builds on PR4 (#8304); until that merges the diff also shows PR4's files, and PR5's own change is the property editor plus two formly wrappers.
Adds the authoring affordance for choosing which operator properties the Form View exposes:
* A toggle on the property panel (shown wherever the feature flag is on) turns on a "choosing" mode, kept in the form-binding service so the canvas toolbar and the Form View preview share the same state.
* In that mode the operator property frame puts a tick box beside each top-level property (ExposePropertyWrapper); ticking it exposes or unexposes that property through the form-binding service.
* An editable-label formly wrapper lets the author rename an exposed field (and its sub-fields) in place.
* The same panel is reused inline in the Form View preview via a persistPlacement flag, so the preview copy never overwrites the docked panel's saved size and position.
### Any related issues, documentation, discussions?
Closes #8017.
Part of the Form View feature (parent issue #8011); builds on PR4 (#8304) for the form-binding service, and on the shared types from #8275.
### How was this PR tested?
Unit tests (vitest): the two new formly wrappers at 100% statements, branches, and functions; the property editor's choosing and expose logic; and the operator frame's tick-box decoration. Coverage is 100% of the lines this PR adds.
### Was this PR authored or co-authored using generative AI tooling?
Yes. Co-authored with Claude (Anthropic), reviewed line by line by the author before submission.
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #8318:
URL: https://github.com/apache/texera/pull/8318#issuecomment-5501773010
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/8318?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 `93.68932%` with `13 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 94.05%. Comparing base ([`246f7bc`](https://app.codecov.io/gh/apache/texera/commit/246f7bc500ff778c02145a34159a70778978fef8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0fba40a`](https://app.codecov.io/gh/apache/texera/commit/0fba40aba39f199fc859ffb2bb63b1ba60a5cd04?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on main.
:white_check_mark: All tests successful. No failed tests found.
| [Files with missing lines](https://app.codecov.io/gh/apache/texera/pull/8318?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...abel-wrapper/editable-label-wrapper.component.html](https://app.codecov.io/gh/apache/texera/pull/8318?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/8318?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ent/property-editor/property-editor.component.html](https://app.codecov.io/gh/apache/texera/pull/8318?src=pr&el=tree&filepath=frontend%2Fsrc%2Fapp%2Fworkspace%2Fcomponent%2Fproperty-editor%2Fproperty-editor.component.html&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZnJvbnRlbmQvc3JjL2FwcC93b3Jrc3BhY2UvY29tcG9uZW50L3Byb3BlcnR5LWVkaXRvci9wcm9wZXJ0eS1lZGl0b3IuY29tcG9uZW50Lmh0bWw=) | 33.33% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/texera/pull/8318?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/8318?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) | 96.77% | [0 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/texera/pull/8318?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...it-frame/operator-property-edit-frame.component.ts](https://app.codecov.io/gh/apache/texera/pull/8318?src=pr&el=tree&filepath=frontend%2Fsrc%2Fapp%2Fworkspace%2Fcomponent%2Fproperty-editor%2Foperator-property-edit-frame%2Foperator-property-edit-frame.component.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZnJvbnRlbmQvc3JjL2FwcC93b3Jrc3BhY2UvY29tcG9uZW50L3Byb3BlcnR5LWVkaXRvci9vcGVyYXRvci1wcm9wZXJ0eS1lZGl0LWZyYW1lL29wZXJhdG9yLXByb3BlcnR5LWVkaXQtZnJhbWUuY29tcG9uZW50LnRz) | 92.30% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/texera/pull/8318?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 #8318 +/- ##
============================================
- Coverage 94.06% 94.05% -0.01%
Complexity 4814 4814
============================================
Files 1196 1201 +5
Lines 48713 48904 +191
Branches 5866 5911 +45
============================================
+ Hits 45824 45999 +175
- Misses 1436 1447 +11
- Partials 1453 1458 +5
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/8318/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/8318/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 [7b74666](https://app.codecov.io/gh/apache/texera/commit/7b746662ee80099598cb154f36db754910ef6dde?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/8318/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 [7b74666](https://app.codecov.io/gh/apache/texera/commit/7b746662ee80099598cb154f36db754910ef6dde?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/8318/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.12% <ø> (ø)` | | Carriedforward from [7b74666](https://app.codecov.io/gh/apache/texera/commit/7b746662ee80099598cb154f36db754910ef6dde?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/8318/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.67% <ø> (ø)` | | Carriedforward from [7b74666](https://app.codecov.io/gh/apache/texera/commit/7b746662ee80099598cb154f36db754910ef6dde?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/8318/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `87.00% <ø> (ø)` | | Carriedforward from [7b74666](https://app.codecov.io/gh/apache/texera/commit/7b746662ee80099598cb154f36db754910ef6dde?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/8318/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `87.91% <ø> (ø)` | | Carriedforward from [7b74666](https://app.codecov.io/gh/apache/texera/commit/7b746662ee80099598cb154f36db754910ef6dde?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/8318/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `96.75% <93.68%> (-0.05%)` | :arrow_down: | |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/8318/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.31% <ø> (ø)` | | Carriedforward from [7b74666](https://app.codecov.io/gh/apache/texera/commit/7b746662ee80099598cb154f36db754910ef6dde?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/8318/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `98.18% <ø> (ø)` | | Carriedforward from [7b74666](https://app.codecov.io/gh/apache/texera/commit/7b746662ee80099598cb154f36db754910ef6dde?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/8318/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 [7b74666](https://app.codecov.io/gh/apache/texera/commit/7b746662ee80099598cb154f36db754910ef6dde?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/8318?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(property-editor): choose which properties the Form View exposes [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8318:
URL: https://github.com/apache/texera/pull/8318#discussion_r3920513303
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.ts:
##########
@@ -74,40 +83,169 @@ import { NzButtonComponent } from "ng-zorro-antd/button";
NzResizeHandlesComponent,
],
})
-export class PropertyEditorComponent implements OnInit, OnDestroy {
+export class PropertyEditorComponent implements OnInit, OnDestroy, OnChanges {
@ViewChild("contentWrapper") contentWrapperRef!: ElementRef;
protected readonly window = window;
id = -1;
- width = 260;
+ width = MIN_PANEL_WIDTH;
height = Math.max(300, window.innerHeight * 0.6);
currentComponent: Type<any> | null = null;
+ /**
+ * Set while an author is choosing which properties the Form View offers.
+ * Forwarded to the operator frame, which puts a tick box beside each property.
+ */
+ @Input() exposeChoosing = false;
+ /**
+ * Whether this panel owns the canvas panel's saved size/position. The Form View mounts
+ * this same component inline in a preview box; only the docked canvas panel persists, so
+ * the preview copy must not overwrite the shared geometry keys.
+ */
+ @Input() persistPlacement = true;
+ /** Set from the toolbar toggle on the operator canvas; the input covers the form view. */
+ private choosingFromToolbar = false;
+
+ /** The choose-what-to-expose affordance appears wherever the feature flag is on: any
+ * workflow can expose inputs to its Form View, independent of the default-view bit. */
+ public get offersFormView(): boolean {
+ return this.config.env.formViewEnabled;
+ }
+
+ public toggleChoosing(): void {
+ this.formBindingService.setChoosing(!this.formBindingService.isChoosing());
+ }
+
+ public get choosing(): boolean {
+ return this.exposeChoosing || this.choosingFromToolbar;
+ }
componentInputs = {};
dragPosition = { x: 0, y: 0 };
- returnPosition = { x: 0, y: 0 };
constructor(
public workflowActionService: WorkflowActionService,
private changeDetectorRef: ChangeDetectorRef,
- private panelService: PanelService
+ private panelService: PanelService,
+ private formBindingService: FormBindingService,
+ private config: GuiConfigService
) {
- const width = localStorage.getItem("right-panel-width");
- if (width) this.width = Number(width);
+ // A stored "0" is a truthy string, so a panel that was closed before a reload used
+ // to come back closed on every load afterwards -- and the button that reopens it is
+ // itself hidden until an operator is selected, so the panel simply looked broken.
+ // Anything narrower than the resize minimum is treated as no stored width at all.
+ const storedWidth = Number(localStorage.getItem("right-panel-width"));
+ if (storedWidth >= MIN_PANEL_WIDTH) this.width = storedWidth;
this.height = Number(localStorage.getItem("right-panel-height")) || this.height;
}
+ /**
+ * The Form View turns tick boxes on by setting this input, and it flips
+ * whenever the author enters or leaves edit mode. The frame builds its formly fields
+ * once, so without remounting here the boxes only appeared if the mode was already on
+ * when the panel opened -- entering edit mode with a step already selected showed none.
+ */
+ ngOnChanges(changes: SimpleChanges): void {
+ if (changes["exposeChoosing"] && !changes["exposeChoosing"].firstChange) {
+ this.remountOperatorFrame();
+ }
+ }
+
ngOnInit(): void {
- const style = localStorage.getItem("right-panel-style");
- if (style) document.getElementById("right-container")!.style.cssText = style;
- const translates = document.getElementById("right-container")!.style.transform;
- const [xOffset, yOffset, _] = calculateTotalTranslate3d(translates);
- this.returnPosition = { x: -xOffset, y: -yOffset };
+ if (this.persistPlacement) {
+ this.restoreSavedPlacement();
+ }
this.registerHighlightEventsHandler();
+ // The toolbar's "choose fields" toggle lives in the service so both the canvas
+ // toolbar and this panel see the same state. Re-emit the frame's inputs when it
+ // changes, so tick boxes appear and disappear without needing a re-selection.
+ this.formBindingService.choosing$.pipe(distinctUntilChanged(), untilDestroyed(this)).subscribe(choosing => {
+ const wasChoosing = this.choosingFromToolbar;
+ this.choosingFromToolbar = choosing;
+ // Only an actual change needs the frame rebuilt. The stream is a BehaviorSubject,
+ // so it replays its current value on subscribe; remounting for that would tear
+ // the panel down during the page's first change-detection pass.
+ if (wasChoosing === choosing || this.currentComponent !== OperatorPropertyEditFrameComponent) {
+ return;
+ }
+ // The frame builds its formly fields once, when it is created, so a new input
+ // alone would not add or remove the tick boxes -- it has to be remounted.
+ //
+ // The restore is in a `finally` and the teardown is not followed by a synchronous
+ // detectChanges: an exception from an unrelated component (the workspace throws
+ // NG0100 in dev mode) used to abort this method between the two assignments,
+ // leaving currentComponent null forever -- and the template hides the whole panel
+ // on `!currentComponent`, so the property editor silently disappeared.
+ this.remountOperatorFrame();
+ });
this.panelService.closePanelStream.pipe(untilDestroyed(this)).subscribe(() => this.closePanel());
this.panelService.resetPanelStream.pipe(untilDestroyed(this)).subscribe(() => {
this.resetPanelPosition();
this.openPanel();
});
}
+ /**
+ * Put the panel back where it was last left, unless that is somewhere unreachable.
+ *
+ * The saved value is the container's raw cssText, which carries the drag transform
+ * with it. A panel dragged past the edge of the window therefore came back off-screen
+ * on every load, and could not be rescued: "reset panels" moved the panel to a home
+ * position that was itself derived from that very transform, so it put the panel
+ * straight back where it already was. An out-of-bounds placement is dropped instead.
+ */
+ private restoreSavedPlacement(): void {
+ const container = document.getElementById("right-container");
+ if (!container) {
+ return;
+ }
+ const saved = localStorage.getItem("right-panel-style");
+ if (!saved) {
+ return;
+ }
+ // Restore the drag offset and nothing else. The saved value is the container's whole
+ // style attribute, so it also carried layout properties: the Form View's
+ // copy of this panel sits inline in a preview box, and it used to save its own
+ // `position: relative` here, which on the operator canvas dropped the docked panel
+ // out of the viewport entirely. Width and height have their own keys, and any style
+ // already poisoned this way is discarded by being ignored.
+ const transform = /transform:\s*([^;]+)/.exec(saved)?.[1]?.trim();
+ if (!transform) {
+ localStorage.removeItem("right-panel-style");
+ return;
+ }
+ const [xOffset, yOffset, _] = calculateTotalTranslate3d(transform);
+ if (this.isOutOfReach(xOffset, yOffset)) {
+ localStorage.removeItem("right-panel-style");
+ return;
+ }
+ container.style.transform = transform;
+ }
+
+ /** True once a drag offset would leave too little of the panel on screen to grab. */
+ private isOutOfReach(xOffset: number, yOffset: number): boolean {
+ const keepVisible = 80;
+ return (
+ Math.abs(xOffset) > Math.max(0, this.window.innerWidth - keepVisible) ||
+ Math.abs(yOffset) > Math.max(0, this.window.innerHeight - keepVisible)
+ );
Review Comment:
Fixed. isOutOfReach now accounts for the panel's home position (docked right, top: 10vh) and its width instead of symmetric bounds, so a rightward offset larger than the panel's own width is rejected. Added a regression test (500px rightward offset on a 260px panel).
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8318:
URL: https://github.com/apache/texera/pull/8318#discussion_r3920514038
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.html:
##########
@@ -60,6 +60,23 @@
nz-menu
id="property-buttons"
[ngClass]="{'shadow': !width}">
+ <!-- Choosing which settings the form offers happens by ticking them in this very
+ panel, so the switch that turns those tick boxes on belongs here rather than in
+ a row of file and delete icons across the toolbar. Only offered on a workflow
Review Comment:
Fixed. Removed the stale "turned the Form View on" wording; the control is flag-only per the always-available design.
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8318:
URL: https://github.com/apache/texera/pull/8318#issuecomment-5501624230
<!-- 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:** `@mengw15`, `@ELin2025`, `@carloea2`
You can notify them by mentioning `@mengw15`, `@ELin2025`, `@carloea2` 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(property-editor): choose which properties the Form View exposes [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 commented on code in PR #8318:
URL: https://github.com/apache/texera/pull/8318#discussion_r3920074262
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.html:
##########
@@ -60,6 +60,23 @@
nz-menu
id="property-buttons"
[ngClass]="{'shadow': !width}">
+ <!-- Choosing which settings the form offers happens by ticking them in this very
+ panel, so the switch that turns those tick boxes on belongs here rather than in
+ a row of file and delete icons across the toolbar. Only offered on a workflow
Review Comment:
Stale sentence from the gating model: the code below (and `offersFormView`) is flag-only per the settled always-available design — there is no per-workflow "turned the Form View on" condition any more.
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 commented on code in PR #8318:
URL: https://github.com/apache/texera/pull/8318#discussion_r3923711618
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.html:
##########
@@ -60,6 +60,24 @@
nz-menu
id="property-buttons"
[ngClass]="{'shadow': !width}">
+ <!-- Choosing which settings the form offers happens by ticking them in this very
+ panel, so the switch that turns those tick boxes on belongs here rather than in
+ a row of file and delete icons across the toolbar. Offered wherever the Form View
+ feature is enabled. -->
+ <button
+ nz-button
+ [nzType]="choosing ? 'primary' : 'text'"
+ class="choose-fields"
+ (click)="toggleChoosing()"
+ *ngIf="width && offersFormView"
+ nz-tooltip
+ nzTooltipPlacement="bottomRight"
+ [attr.aria-label]="choosing ? 'Done choosing' : 'Choose what the form offers'"
+ [nzTooltipTitle]="choosing ? 'Done choosing' : 'Choose what the form offers'">
Review Comment:
The off-state copy is easy to misread: the property panel is itself a form, so "the form" can be read as this very panel — and this is the feature's entry point, yet it is the one string here that never names the Form View (the tick-box strings do). Consider e.g. "Choose Form View fields" / "Done choosing fields". Minor, same theme: on hover sighted users get the static "Show this on the Form View" while the aria-label carries the property name — reusing the dynamic string for the title would serve both. Not blocking.
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.ts:
##########
@@ -74,40 +84,191 @@ import { NzButtonComponent } from "ng-zorro-antd/button";
NzResizeHandlesComponent,
],
})
-export class PropertyEditorComponent implements OnInit, OnDestroy {
+export class PropertyEditorComponent implements OnInit, OnDestroy, OnChanges {
@ViewChild("contentWrapper") contentWrapperRef!: ElementRef;
protected readonly window = window;
id = -1;
- width = 260;
+ width = MIN_PANEL_WIDTH;
height = Math.max(300, window.innerHeight * 0.6);
currentComponent: Type<any> | null = null;
+ /**
+ * Set while an author is choosing which properties the Form View offers.
+ * Forwarded to the operator frame, which puts a tick box beside each property.
+ */
+ @Input() exposeChoosing = false;
+ /**
+ * Whether this panel owns the canvas panel's saved size/position. The Form View mounts
+ * this same component inline in a preview box; only the docked canvas panel persists, so
+ * the preview copy must not overwrite the shared geometry keys.
+ */
+ @Input() persistPlacement = true;
+ /** Set from the toolbar toggle on the operator canvas; the input covers the form view. */
+ private choosingFromToolbar = false;
+
+ /** The choose-what-to-expose affordance appears wherever the feature flag is on: any
+ * workflow can expose inputs to its Form View, independent of the default-view bit. */
+ public get offersFormView(): boolean {
Review Comment:
Naming nit: `offersFormView` reads like a per-workflow capability (the retired gating model's shape), so at the template call site nothing hints this is the feature flag — something like `formViewFeatureEnabled` would make the gating visible where it is used. Not blocking.
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8318:
URL: https://github.com/apache/texera/pull/8318#discussion_r3929095970
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.html:
##########
@@ -60,6 +60,24 @@
nz-menu
id="property-buttons"
[ngClass]="{'shadow': !width}">
+ <!-- Choosing which settings the form offers happens by ticking them in this very
+ panel, so the switch that turns those tick boxes on belongs here rather than in
+ a row of file and delete icons across the toolbar. Offered wherever the Form View
+ feature is enabled. -->
+ <button
+ nz-button
+ [nzType]="choosing ? 'primary' : 'text'"
+ class="choose-fields"
+ (click)="toggleChoosing()"
+ *ngIf="width && offersFormView"
+ nz-tooltip
+ nzTooltipPlacement="bottomRight"
+ [attr.aria-label]="choosing ? 'Done choosing' : 'Choose what the form offers'"
+ [nzTooltipTitle]="choosing ? 'Done choosing' : 'Choose what the form offers'">
Review Comment:
Applied. The button now reads "Choose Form View fields" / "Done choosing fields" so it names the feature and can't be read as this panel, and the expose tick box's title now reuses the same dynamic string as its aria-label ("Show <property> on the Form View").
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.ts:
##########
@@ -74,40 +84,191 @@ import { NzButtonComponent } from "ng-zorro-antd/button";
NzResizeHandlesComponent,
],
})
-export class PropertyEditorComponent implements OnInit, OnDestroy {
+export class PropertyEditorComponent implements OnInit, OnDestroy, OnChanges {
@ViewChild("contentWrapper") contentWrapperRef!: ElementRef;
protected readonly window = window;
id = -1;
- width = 260;
+ width = MIN_PANEL_WIDTH;
height = Math.max(300, window.innerHeight * 0.6);
currentComponent: Type<any> | null = null;
+ /**
+ * Set while an author is choosing which properties the Form View offers.
+ * Forwarded to the operator frame, which puts a tick box beside each property.
+ */
+ @Input() exposeChoosing = false;
+ /**
+ * Whether this panel owns the canvas panel's saved size/position. The Form View mounts
+ * this same component inline in a preview box; only the docked canvas panel persists, so
+ * the preview copy must not overwrite the shared geometry keys.
+ */
+ @Input() persistPlacement = true;
+ /** Set from the toolbar toggle on the operator canvas; the input covers the form view. */
+ private choosingFromToolbar = false;
+
+ /** The choose-what-to-expose affordance appears wherever the feature flag is on: any
+ * workflow can expose inputs to its Form View, independent of the default-view bit. */
+ public get offersFormView(): boolean {
Review Comment:
Applied. Renamed offersFormView to formViewFeatureEnabled so the flag nature is visible at the call site.
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8318:
URL: https://github.com/apache/texera/pull/8318#discussion_r3929102524
##########
frontend/src/app/common/formly/formly-config.ts:
##########
@@ -92,6 +94,8 @@ export const TEXERA_FORMLY_CONFIG = {
wrappers: [
{ name: "preset-wrapper", component: PresetWrapperComponent },
{ name: "collab-wrapper", component: CollabWrapperComponent },
+ { name: "expose-property-wrapper", component: ExposePropertyWrapperComponent },
+ { name: "editable-label-wrapper", component: EditableLabelWrapperComponent },
Review Comment:
Moved out. This wrapper had no production consumer in this PR; it belongs with the render PR that calls its decorate, so I removed it here (its 4 files + the formly-config registration). This PR now carries only the wired expose-property-wrapper.
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 merged PR #8318:
URL: https://github.com/apache/texera/pull/8318
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8318:
URL: https://github.com/apache/texera/pull/8318#discussion_r3920489296
##########
frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html:
##########
@@ -0,0 +1,55 @@
+<!--
+ 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)"
+ [title]="'Shown above this box. Empty keeps ' + props.schemaLabel" />
+ <!-- An input the reader can only lose by removing it altogether has no eye: offering
+ one here would be a second place to decide the same thing. -->
+ <button
+ *ngIf="props.canHide !== false"
+ type="button"
+ class="lbl-eye"
+ [class.off]="props.authorHidden"
+ (click)="onToggleHidden()"
+ [title]="props.authorHidden ? 'Hidden from the form' : 'Shown on the form'">
+ <i
+ nz-icon
+ [nzType]="props.authorHidden ? 'eye-invisible' : 'eye'"
+ nzTheme="outline"></i>
+ </button>
+</div>
+
+<!-- Everyone else just reads it. -->
+<label
+ class="lbl-static"
+ *ngIf="!props.authoring && (props.authorName || props.schemaLabel)">
+ {{ props.authorName || props.schemaLabel }}
Review Comment:
Fixed. The reader-mode static label now carries [attr.for]="id" so assistive technology announces the generated input with this label.
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8318:
URL: https://github.com/apache/texera/pull/8318#discussion_r3920488247
##########
frontend/src/app/common/formly/formly-config.ts:
##########
@@ -92,6 +94,8 @@ export const TEXERA_FORMLY_CONFIG = {
wrappers: [
{ name: "preset-wrapper", component: PresetWrapperComponent },
{ name: "collab-wrapper", component: CollabWrapperComponent },
+ { name: "expose-property-wrapper", component: ExposePropertyWrapperComponent },
+ { name: "editable-label-wrapper", component: EditableLabelWrapperComponent },
Review Comment:
Intentional layering in this stacked series. This PR adds the two authoring wrappers and wires the expose tick-box into the property editor; the editable-label wrapper is applied to Form View fields in a later PR of the stack (the one rendering the exposed inputs), where the rename/hide callbacks to setFieldOverride live. Registering it here keeps that later PR focused on the rendering path.
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8318:
URL: https://github.com/apache/texera/pull/8318#discussion_r3920512089
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.html:
##########
@@ -60,6 +60,23 @@
nz-menu
id="property-buttons"
[ngClass]="{'shadow': !width}">
+ <!-- Choosing which settings the form offers happens by ticking them in this very
+ panel, so the switch that turns those tick boxes on belongs here rather than in
+ a row of file and delete icons across the toolbar. Only offered on a workflow
+ whose author turned the Form View on. -->
+ <button
+ nz-button
+ [nzType]="choosing ? 'primary' : 'text'"
+ class="choose-fields"
+ (click)="toggleChoosing()"
+ *ngIf="width && offersFormView"
Review Comment:
Fixed. The icon-only toggle now has a state-dependent [attr.aria-label] matching the tooltip.
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on PR #8318:
URL: https://github.com/apache/texera/pull/8318#issuecomment-5515421688
/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(property-editor): choose which properties the Form View exposes [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8318:
URL: https://github.com/apache/texera/pull/8318#discussion_r3919482684
##########
frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html:
##########
@@ -0,0 +1,55 @@
+<!--
+ 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)"
+ [title]="'Shown above this box. Empty keeps ' + props.schemaLabel" />
+ <!-- An input the reader can only lose by removing it altogether has no eye: offering
+ one here would be a second place to decide the same thing. -->
+ <button
+ *ngIf="props.canHide !== false"
+ type="button"
+ class="lbl-eye"
+ [class.off]="props.authorHidden"
+ (click)="onToggleHidden()"
+ [title]="props.authorHidden ? 'Hidden from the form' : 'Shown on the form'">
+ <i
+ nz-icon
+ [nzType]="props.authorHidden ? 'eye-invisible' : 'eye'"
+ nzTheme="outline"></i>
+ </button>
+</div>
+
+<!-- Everyone else just reads it. -->
+<label
+ class="lbl-static"
+ *ngIf="!props.authoring && (props.authorName || props.schemaLabel)">
+ {{ props.authorName || props.schemaLabel }}
Review Comment:
Blanking Formly's normal label and rendering this standalone `<label>` removes the control association because it has no `for` attribute. In reader mode, assistive technology will therefore announce the generated input without its author-defined/schema label. Associate this label with Formly's generated field id.
##########
frontend/src/app/common/formly/formly-config.ts:
##########
@@ -92,6 +94,8 @@ export const TEXERA_FORMLY_CONFIG = {
wrappers: [
{ name: "preset-wrapper", component: PresetWrapperComponent },
{ name: "collab-wrapper", component: CollabWrapperComponent },
+ { name: "expose-property-wrapper", component: ExposePropertyWrapperComponent },
+ { name: "editable-label-wrapper", component: EditableLabelWrapperComponent },
Review Comment:
Registering this wrapper only makes its name available to Formly; it does not apply it to any field. There is no production call to `EditableLabelWrapperComponent.decorate` (the only calls are in its spec), so the PR's advertised rename/hide UI can never appear. Wire the wrapper into the Form View field-generation path with callbacks to `setFieldOverride`, or defer the claimed behavior and component to the PR that does so.
##########
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts:
##########
@@ -1341,13 +1353,28 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On
};
}
+ // A tick box beside each TOP-LEVEL property only. The map callback runs for every
+ // field at every depth, so without this an array-of-objects property sprouted one on
+ // the array, each item and each nested field. Membership in the schema's own
+ // `properties` distinguishes a top-level field from a same-named nested one.
+ const isTopLevel = typeof mappedField.key === "string" && rootPropertyNames.has(mappedField.key);
Review Comment:
The new frame test covers only a flat, single-property schema, so it does not verify the key behavior introduced here: nested array/object fields must not receive expose toggles. Add a regression schema with nested fields (including a nested key that matches a root property name), assert only root fields are decorated, and adjust the depth check if that case fails.
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.ts:
##########
@@ -74,40 +83,169 @@ import { NzButtonComponent } from "ng-zorro-antd/button";
NzResizeHandlesComponent,
],
})
-export class PropertyEditorComponent implements OnInit, OnDestroy {
+export class PropertyEditorComponent implements OnInit, OnDestroy, OnChanges {
@ViewChild("contentWrapper") contentWrapperRef!: ElementRef;
protected readonly window = window;
id = -1;
- width = 260;
+ width = MIN_PANEL_WIDTH;
height = Math.max(300, window.innerHeight * 0.6);
currentComponent: Type<any> | null = null;
+ /**
+ * Set while an author is choosing which properties the Form View offers.
+ * Forwarded to the operator frame, which puts a tick box beside each property.
+ */
+ @Input() exposeChoosing = false;
+ /**
+ * Whether this panel owns the canvas panel's saved size/position. The Form View mounts
+ * this same component inline in a preview box; only the docked canvas panel persists, so
+ * the preview copy must not overwrite the shared geometry keys.
+ */
+ @Input() persistPlacement = true;
+ /** Set from the toolbar toggle on the operator canvas; the input covers the form view. */
+ private choosingFromToolbar = false;
+
+ /** The choose-what-to-expose affordance appears wherever the feature flag is on: any
+ * workflow can expose inputs to its Form View, independent of the default-view bit. */
+ public get offersFormView(): boolean {
+ return this.config.env.formViewEnabled;
+ }
+
+ public toggleChoosing(): void {
+ this.formBindingService.setChoosing(!this.formBindingService.isChoosing());
+ }
+
+ public get choosing(): boolean {
+ return this.exposeChoosing || this.choosingFromToolbar;
+ }
componentInputs = {};
dragPosition = { x: 0, y: 0 };
- returnPosition = { x: 0, y: 0 };
constructor(
public workflowActionService: WorkflowActionService,
private changeDetectorRef: ChangeDetectorRef,
- private panelService: PanelService
+ private panelService: PanelService,
+ private formBindingService: FormBindingService,
+ private config: GuiConfigService
) {
- const width = localStorage.getItem("right-panel-width");
- if (width) this.width = Number(width);
+ // A stored "0" is a truthy string, so a panel that was closed before a reload used
+ // to come back closed on every load afterwards -- and the button that reopens it is
+ // itself hidden until an operator is selected, so the panel simply looked broken.
+ // Anything narrower than the resize minimum is treated as no stored width at all.
+ const storedWidth = Number(localStorage.getItem("right-panel-width"));
+ if (storedWidth >= MIN_PANEL_WIDTH) this.width = storedWidth;
this.height = Number(localStorage.getItem("right-panel-height")) || this.height;
Review Comment:
A closed panel persists `height = 65` in `ngOnDestroy`. Although the new width check rejects the corresponding stored width of zero, this line still restores 65px, so the panel reopens below its declared 300px minimum. Validate the stored height before applying it (and add the height assertion to the closed-panel regression test).
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.html:
##########
@@ -60,6 +60,23 @@
nz-menu
id="property-buttons"
[ngClass]="{'shadow': !width}">
+ <!-- Choosing which settings the form offers happens by ticking them in this very
+ panel, so the switch that turns those tick boxes on belongs here rather than in
+ a row of file and delete icons across the toolbar. Only offered on a workflow
+ whose author turned the Form View on. -->
+ <button
+ nz-button
+ [nzType]="choosing ? 'primary' : 'text'"
+ class="choose-fields"
+ (click)="toggleChoosing()"
+ *ngIf="width && offersFormView"
Review Comment:
This new icon-only toggle has no accessible name; an `nz-tooltip` provides visual help but is not a button label for assistive technology. Add the same state-dependent text as an `aria-label`.
##########
frontend/src/app/common/formly/expose-property-wrapper/expose-property-wrapper.component.html:
##########
@@ -0,0 +1,32 @@
+<!--
+ 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.
+-->
+
+<div class="expose-row choosing">
+ <label
+ class="expose-box"
+ title="Show this on the Form View">
+ <input
+ type="checkbox"
+ [checked]="props.exposed"
+ (change)="onToggle($event)" />
Review Comment:
The checkbox has no accessible name. Its wrapping label contains no text, and the label's `title` is not used as the nested input's label, so screen-reader users cannot tell which property this checkbox exposes. Give the input a property-specific `aria-label`.
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.ts:
##########
@@ -74,40 +83,169 @@ import { NzButtonComponent } from "ng-zorro-antd/button";
NzResizeHandlesComponent,
],
})
-export class PropertyEditorComponent implements OnInit, OnDestroy {
+export class PropertyEditorComponent implements OnInit, OnDestroy, OnChanges {
@ViewChild("contentWrapper") contentWrapperRef!: ElementRef;
protected readonly window = window;
id = -1;
- width = 260;
+ width = MIN_PANEL_WIDTH;
height = Math.max(300, window.innerHeight * 0.6);
currentComponent: Type<any> | null = null;
+ /**
+ * Set while an author is choosing which properties the Form View offers.
+ * Forwarded to the operator frame, which puts a tick box beside each property.
+ */
+ @Input() exposeChoosing = false;
+ /**
+ * Whether this panel owns the canvas panel's saved size/position. The Form View mounts
+ * this same component inline in a preview box; only the docked canvas panel persists, so
+ * the preview copy must not overwrite the shared geometry keys.
+ */
+ @Input() persistPlacement = true;
+ /** Set from the toolbar toggle on the operator canvas; the input covers the form view. */
+ private choosingFromToolbar = false;
+
+ /** The choose-what-to-expose affordance appears wherever the feature flag is on: any
+ * workflow can expose inputs to its Form View, independent of the default-view bit. */
+ public get offersFormView(): boolean {
+ return this.config.env.formViewEnabled;
+ }
+
+ public toggleChoosing(): void {
+ this.formBindingService.setChoosing(!this.formBindingService.isChoosing());
+ }
+
+ public get choosing(): boolean {
+ return this.exposeChoosing || this.choosingFromToolbar;
+ }
componentInputs = {};
dragPosition = { x: 0, y: 0 };
- returnPosition = { x: 0, y: 0 };
constructor(
public workflowActionService: WorkflowActionService,
private changeDetectorRef: ChangeDetectorRef,
- private panelService: PanelService
+ private panelService: PanelService,
+ private formBindingService: FormBindingService,
+ private config: GuiConfigService
) {
- const width = localStorage.getItem("right-panel-width");
- if (width) this.width = Number(width);
+ // A stored "0" is a truthy string, so a panel that was closed before a reload used
+ // to come back closed on every load afterwards -- and the button that reopens it is
+ // itself hidden until an operator is selected, so the panel simply looked broken.
+ // Anything narrower than the resize minimum is treated as no stored width at all.
+ const storedWidth = Number(localStorage.getItem("right-panel-width"));
+ if (storedWidth >= MIN_PANEL_WIDTH) this.width = storedWidth;
this.height = Number(localStorage.getItem("right-panel-height")) || this.height;
}
+ /**
+ * The Form View turns tick boxes on by setting this input, and it flips
+ * whenever the author enters or leaves edit mode. The frame builds its formly fields
+ * once, so without remounting here the boxes only appeared if the mode was already on
+ * when the panel opened -- entering edit mode with a step already selected showed none.
+ */
+ ngOnChanges(changes: SimpleChanges): void {
+ if (changes["exposeChoosing"] && !changes["exposeChoosing"].firstChange) {
+ this.remountOperatorFrame();
+ }
+ }
+
ngOnInit(): void {
- const style = localStorage.getItem("right-panel-style");
- if (style) document.getElementById("right-container")!.style.cssText = style;
- const translates = document.getElementById("right-container")!.style.transform;
- const [xOffset, yOffset, _] = calculateTotalTranslate3d(translates);
- this.returnPosition = { x: -xOffset, y: -yOffset };
+ if (this.persistPlacement) {
+ this.restoreSavedPlacement();
+ }
this.registerHighlightEventsHandler();
+ // The toolbar's "choose fields" toggle lives in the service so both the canvas
+ // toolbar and this panel see the same state. Re-emit the frame's inputs when it
+ // changes, so tick boxes appear and disappear without needing a re-selection.
+ this.formBindingService.choosing$.pipe(distinctUntilChanged(), untilDestroyed(this)).subscribe(choosing => {
+ const wasChoosing = this.choosingFromToolbar;
+ this.choosingFromToolbar = choosing;
+ // Only an actual change needs the frame rebuilt. The stream is a BehaviorSubject,
+ // so it replays its current value on subscribe; remounting for that would tear
+ // the panel down during the page's first change-detection pass.
+ if (wasChoosing === choosing || this.currentComponent !== OperatorPropertyEditFrameComponent) {
+ return;
+ }
+ // The frame builds its formly fields once, when it is created, so a new input
+ // alone would not add or remove the tick boxes -- it has to be remounted.
+ //
+ // The restore is in a `finally` and the teardown is not followed by a synchronous
+ // detectChanges: an exception from an unrelated component (the workspace throws
+ // NG0100 in dev mode) used to abort this method between the two assignments,
+ // leaving currentComponent null forever -- and the template hides the whole panel
+ // on `!currentComponent`, so the property editor silently disappeared.
+ this.remountOperatorFrame();
+ });
this.panelService.closePanelStream.pipe(untilDestroyed(this)).subscribe(() => this.closePanel());
this.panelService.resetPanelStream.pipe(untilDestroyed(this)).subscribe(() => {
this.resetPanelPosition();
this.openPanel();
});
}
+ /**
+ * Put the panel back where it was last left, unless that is somewhere unreachable.
+ *
+ * The saved value is the container's raw cssText, which carries the drag transform
+ * with it. A panel dragged past the edge of the window therefore came back off-screen
+ * on every load, and could not be rescued: "reset panels" moved the panel to a home
+ * position that was itself derived from that very transform, so it put the panel
+ * straight back where it already was. An out-of-bounds placement is dropped instead.
+ */
+ private restoreSavedPlacement(): void {
+ const container = document.getElementById("right-container");
+ if (!container) {
+ return;
+ }
+ const saved = localStorage.getItem("right-panel-style");
+ if (!saved) {
+ return;
+ }
+ // Restore the drag offset and nothing else. The saved value is the container's whole
+ // style attribute, so it also carried layout properties: the Form View's
+ // copy of this panel sits inline in a preview box, and it used to save its own
+ // `position: relative` here, which on the operator canvas dropped the docked panel
+ // out of the viewport entirely. Width and height have their own keys, and any style
+ // already poisoned this way is discarded by being ignored.
+ const transform = /transform:\s*([^;]+)/.exec(saved)?.[1]?.trim();
+ if (!transform) {
+ localStorage.removeItem("right-panel-style");
+ return;
+ }
+ const [xOffset, yOffset, _] = calculateTotalTranslate3d(transform);
+ if (this.isOutOfReach(xOffset, yOffset)) {
+ localStorage.removeItem("right-panel-style");
+ return;
+ }
+ container.style.transform = transform;
+ }
+
+ /** True once a drag offset would leave too little of the panel on screen to grab. */
+ private isOutOfReach(xOffset: number, yOffset: number): boolean {
+ const keepVisible = 80;
+ return (
+ Math.abs(xOffset) > Math.max(0, this.window.innerWidth - keepVisible) ||
+ Math.abs(yOffset) > Math.max(0, this.window.innerHeight - keepVisible)
+ );
Review Comment:
These symmetric offset bounds do not match a panel anchored at `right: 0; top: 10vh`. For example, a 260px panel restored with `xOffset = 500` on a 1024px viewport is entirely beyond the right edge, but this check accepts it because `500 < 944`. Account for the panel's home position and dimensions so resized viewports cannot restore an unreachable panel.
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8318:
URL: https://github.com/apache/texera/pull/8318#discussion_r3921139869
##########
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts:
##########
@@ -1341,13 +1353,28 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On
};
}
+ // A tick box beside each TOP-LEVEL property only. The map callback runs for every
+ // field at every depth, so without this an array-of-objects property sprouted one on
+ // the array, each item and each nested field. Membership in the schema's own
+ // `properties` distinguishes a top-level field from a same-named nested one.
+ const isTopLevel = typeof mappedField.key === "string" && rootPropertyNames.has(mappedField.key);
Review Comment:
Fixed. Moved the tick-box decoration off the per-depth formly map onto the root field group, so nested array/object fields can no longer receive a toggle (the map ran at every depth and could not tell a nested field from a same-named root one). Added a regression test with a nested object whose key collides with a root property, asserting only the root fields are decorated.
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8318:
URL: https://github.com/apache/texera/pull/8318#discussion_r3920510610
##########
frontend/src/app/common/formly/expose-property-wrapper/expose-property-wrapper.component.html:
##########
@@ -0,0 +1,32 @@
+<!--
+ 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.
+-->
+
+<div class="expose-row choosing">
+ <label
+ class="expose-box"
+ title="Show this on the Form View">
+ <input
+ type="checkbox"
+ [checked]="props.exposed"
+ (change)="onToggle($event)" />
Review Comment:
Fixed. The checkbox now has a property-specific [attr.aria-label] ("Show <property> on the Form View").
##########
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts:
##########
@@ -1341,13 +1353,28 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On
};
}
+ // A tick box beside each TOP-LEVEL property only. The map callback runs for every
+ // field at every depth, so without this an array-of-objects property sprouted one on
+ // the array, each item and each nested field. Membership in the schema's own
+ // `properties` distinguishes a top-level field from a same-named nested one.
+ const isTopLevel = typeof mappedField.key === "string" && rootPropertyNames.has(mappedField.key);
Review Comment:
Fixed. Moved the tick-box decoration off the per-depth formly map onto the root field group, so nested array/object fields can no longer receive a toggle (the map ran at every depth and could not tell a nested field from a same-named root one). Added a regression test with a nested object whose key collides with a root property, asserting only root fields are decorated.
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8318:
URL: https://github.com/apache/texera/pull/8318#discussion_r3920511384
##########
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts:
##########
@@ -1341,13 +1353,28 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On
};
}
+ // A tick box beside each TOP-LEVEL property only. The map callback runs for every
+ // field at every depth, so without this an array-of-objects property sprouted one on
+ // the array, each item and each nested field. Membership in the schema's own
+ // `properties` distinguishes a top-level field from a same-named nested one.
+ const isTopLevel = typeof mappedField.key === "string" && rootPropertyNames.has(mappedField.key);
Review Comment:
Fixed. Moved the tick-box decoration off the per-depth formly map onto the root field group, so nested array/object fields can no longer receive a toggle (the map ran at every depth and could not tell a nested field from a same-named root one). Added a regression test with a nested object whose key collides with a root property, asserting only root fields are decorated.
--
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(property-editor): choose which properties the Form View exposes [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on code in PR #8318:
URL: https://github.com/apache/texera/pull/8318#discussion_r3920519245
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.ts:
##########
@@ -74,40 +83,169 @@ import { NzButtonComponent } from "ng-zorro-antd/button";
NzResizeHandlesComponent,
],
})
-export class PropertyEditorComponent implements OnInit, OnDestroy {
+export class PropertyEditorComponent implements OnInit, OnDestroy, OnChanges {
@ViewChild("contentWrapper") contentWrapperRef!: ElementRef;
protected readonly window = window;
id = -1;
- width = 260;
+ width = MIN_PANEL_WIDTH;
height = Math.max(300, window.innerHeight * 0.6);
currentComponent: Type<any> | null = null;
+ /**
+ * Set while an author is choosing which properties the Form View offers.
+ * Forwarded to the operator frame, which puts a tick box beside each property.
+ */
+ @Input() exposeChoosing = false;
+ /**
+ * Whether this panel owns the canvas panel's saved size/position. The Form View mounts
+ * this same component inline in a preview box; only the docked canvas panel persists, so
+ * the preview copy must not overwrite the shared geometry keys.
+ */
+ @Input() persistPlacement = true;
+ /** Set from the toolbar toggle on the operator canvas; the input covers the form view. */
+ private choosingFromToolbar = false;
+
+ /** The choose-what-to-expose affordance appears wherever the feature flag is on: any
+ * workflow can expose inputs to its Form View, independent of the default-view bit. */
+ public get offersFormView(): boolean {
+ return this.config.env.formViewEnabled;
+ }
+
+ public toggleChoosing(): void {
+ this.formBindingService.setChoosing(!this.formBindingService.isChoosing());
+ }
+
+ public get choosing(): boolean {
+ return this.exposeChoosing || this.choosingFromToolbar;
+ }
componentInputs = {};
dragPosition = { x: 0, y: 0 };
- returnPosition = { x: 0, y: 0 };
constructor(
public workflowActionService: WorkflowActionService,
private changeDetectorRef: ChangeDetectorRef,
- private panelService: PanelService
+ private panelService: PanelService,
+ private formBindingService: FormBindingService,
+ private config: GuiConfigService
) {
- const width = localStorage.getItem("right-panel-width");
- if (width) this.width = Number(width);
+ // A stored "0" is a truthy string, so a panel that was closed before a reload used
+ // to come back closed on every load afterwards -- and the button that reopens it is
+ // itself hidden until an operator is selected, so the panel simply looked broken.
+ // Anything narrower than the resize minimum is treated as no stored width at all.
+ const storedWidth = Number(localStorage.getItem("right-panel-width"));
+ if (storedWidth >= MIN_PANEL_WIDTH) this.width = storedWidth;
this.height = Number(localStorage.getItem("right-panel-height")) || this.height;
Review Comment:
Fixed. closePanel() stores the closed pair (width 0, height 65) together, so a sub-minimum stored width means the height beside it is the collapsed 65. Height is now restored only alongside a valid width; a closed panel reopens at defaults instead of a 65px sliver. Added the height assertion to the closed-panel regression test.
--
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