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

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

Reply via email to