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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to