fix: Floating panel and dialog bugs (#4708)

## Description - [x] fix maximize/minimize size/positioning - open, move, maximize, minimize - should be in the last position after moving - [x] fixes #4681 - [x] fixes #4684 ## Steps for reproduction 1. click button 2. expect xyz ## Code Review - [ ] hi @kof, I need you to do - conceptual review (architecture, feature-correctness) - detailed review (read every line) - test it on preview ## Before requesting a review - [ ] made a self-review - [ ] added inline comments where things may be not obvious (the "why", not "what") ## Before merging - [ ] tested locally and on preview environment (preview dev login: 0000) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env` file
2025-03-14 09:57:02 +00:00 · 2025-01-09 16:29:08 +00:00
parent a09d12abe0
commit 5fa6c0354f
5 changed files with 153 additions and 105 deletions
--- a/apps/builder/app/builder/features/style-panel/shared/css-value-input/value-editor-dialog.tsx
+++ b/apps/builder/app/builder/features/style-panel/shared/css-value-input/value-editor-dialog.tsx
@ -92,7 +92,7 @@ export const ValueEditorDialog = ({
      title="CSS Value"
      placement="bottom"
      height={200}
-      width={Number(rawTheme.sizes.sidebarWidth)}
+      width={Number.parseFloat(rawTheme.sizes.sidebarWidth)}
      content={
        <CssFragmentEditorContent
          autoFocus
--- a/packages/css-data/src/property-parsers/linear-gradient.ts
+++ b/packages/css-data/src/property-parsers/linear-gradient.ts
@ -131,7 +131,7 @@ const mapPercenTageOrDimentionToUnit = (

  return {
    type: "unit",
-    value: parseFloat(node.value),
+    value: Number.parseFloat(node.value),
    unit: node.type === "Percentage" ? "%" : (node.unit as Unit),
  };
 };
--- a/packages/design-system/src/components/dialog.tsx
+++ b/packages/design-system/src/components/dialog.tsx
@ -8,6 +8,9 @@ import {
  createContext,
  useContext,
  useState,
+  useEffect,
+  useCallback,
+  type RefObject,
 } from "react";
 import * as Primitive from "@radix-ui/react-dialog";
 import { css, theme, type CSS } from "../stitches.config";
@ -120,30 +123,87 @@ type Point = { x: number; y: number };
 type Size = { width: number; height: number };
 type Rect = Point & Size;

+const centeredContent = {
+  top: "50%",
+  left: "50%",
+  transform: "translate(-50%, -50%)",
+  width: "100vw",
+  height: "100vh",
+} as const;
+
 type UseDraggableProps = {
-  isMaximized?: boolean;
+  isMaximized: boolean;
  minWidth?: number;
  minHeight?: number;
 } & Partial<Rect>;

 const useDraggable = ({
-  x,
-  y,
  width,
  height,
  minHeight,
  minWidth,
+  isMaximized,
+  ...props
 }: UseDraggableProps) => {
-  const { isMaximized } = useContext(DialogContext);
-  const initialDataRef = useRef<
+  const [x, setX] = useState(props.x);
+  const [y, setY] = useState(props.y);
+
+  const lastDragDataRef = useRef<
    | undefined
    | {
        point: Point;
        rect: Rect;
      }
  >(undefined);
+
  const ref = useRef<HTMLDivElement | null>(null);

+  const calcStyle = useCallback(() => {
+    const style: CSSProperties = isMaximized
+      ? centeredContent
+      : {
+          ...centeredContent,
+          width,
+          height,
+        };
+
+    if (minWidth !== undefined) {
+      style.minWidth = minWidth;
+    }
+    if (minHeight !== undefined) {
+      style.minHeight = minHeight;
+    }
+
+    if (isMaximized === false) {
+      if (x !== undefined) {
+        style.left = x;
+        style.transform = "none";
+      }
+      if (y !== undefined) {
+        style.top = y;
+        style.transform = "none";
+      }
+    }
+    return style;
+  }, [x, y, width, height, isMaximized, minWidth, minHeight]);
+
+  const [style, setStyle] = useState(calcStyle());
+
+  useEffect(() => {
+    setStyle(calcStyle());
+  }, [calcStyle]);
+
+  useEffect(() => {
+    if (lastDragDataRef.current) {
+      // Until user draggs, we need component props to define the position, because floating panel needs to adjust it after rendering.
+      // We don't want to use the props x/y value after user has dragged manually. At this point position is defined
+      // by drag interaction and props can't override it, otherwise position will jump for unpredictable reasons, e.g. when parent decides to update.
+      return;
+    }
+    setX(props.x);
+    setY(props.y);
+  }, [props.x, props.y]);
+
  const handleDragStart: DragEventHandler = (event) => {
    const target = ref.current;
    if (target === null) {
@ -157,7 +217,7 @@ const useDraggable = ({
    target.style.left = `${rect.x}px`;
    target.style.top = `${rect.y}px`;
    target.style.transform = "none";
-    initialDataRef.current = {
+    lastDragDataRef.current = {
      point: { x: event.pageX, y: event.pageY },
      rect,
    };
@ -169,12 +229,12 @@ const useDraggable = ({
    if (
      event.pageX <= 0 ||
      event.pageY <= 0 ||
-      initialDataRef.current === undefined ||
+      lastDragDataRef.current === undefined ||
      target === null
    ) {
      return;
    }
-    const { rect, point } = initialDataRef.current;
+    const { rect, point } = lastDragDataRef.current;
    const movementX = point.x - event.pageX;
    const movementY = point.y - event.pageY;
    let left = Math.max(rect.x - movementX, 0);
@ -186,80 +246,49 @@ const useDraggable = ({
    target.style.top = `${top}px`;
  };

-  const style: CSSProperties = isMaximized
-    ? {
-        ...centeredContent,
-        width: "100vw",
-        height: "100vh",
-      }
-    : {
-        ...centeredContent,
-        width,
-        height,
-      };
+  const handleDragEnd: DragEventHandler = () => {
+    const target = ref.current;
+    if (target === null) {
+      return;
+    }
+    const rect = target.getBoundingClientRect();
+    setX(rect.x);
+    setY(rect.y);
+  };

-  if (minWidth !== undefined) {
-    style.minWidth = minWidth;
-  }
-  if (minHeight !== undefined) {
-    style.minHeight = minHeight;
-  }
-  if (isMaximized === false) {
-    if (x !== undefined) {
-      style.left = x;
-      delete style.transform;
-    }
-    if (y !== undefined) {
-      style.top = y;
-      delete style.transform;
-    }
-  }
  return {
    onDragStart: handleDragStart,
    onDrag: handleDrag,
+    onDragEnd: handleDragEnd,
    style,
    ref,
  };
 };

 // This is needed to prevent pointer events on the iframe from interfering with dragging and resizing.
-const useSetPointerEvents = () => {
+const useSetPointerEvents = (elementRef: RefObject<HTMLElement | null>) => {
  const { enableCanvasPointerEvents, disableCanvasPointerEvents } =
    useDisableCanvasPointerEvents();

-  const setPointerEvents = (value: string) => {
-    return () => {
-      value === "none"
-        ? disableCanvasPointerEvents()
-        : enableCanvasPointerEvents();
-      // RAF is needed otherwise dragstart event won't fire because of pointer-events: none
-      requestAnimationFrame(() => {
-        if (element) {
-          element.style.pointerEvents = value;
-        }
-      });
-    };
-  };
-
-  const [element, ref] = useResize({
-    onResizeStart: setPointerEvents("none"),
-    onResizeEnd: setPointerEvents("auto"),
-  });
-
-  const { resize, draggable } = useContext(DialogContext);
-
-  if (resize === "none" && draggable !== true) {
-    return {};
-  }
-
-  return {
-    ref,
-    onDragStartCapture: setPointerEvents("none"),
-    onDragEndCapture: setPointerEvents("auto"),
-  };
+  return useCallback(
+    (value: string) => {
+      return () => {
+        value === "none"
+          ? disableCanvasPointerEvents()
+          : enableCanvasPointerEvents();
+        // RAF is needed otherwise dragstart event won't fire because of pointer-events: none
+        requestAnimationFrame(() => {
+          if (elementRef.current) {
+            elementRef.current.style.pointerEvents = value;
+          }
+        });
+      };
+    },
+    [elementRef, enableCanvasPointerEvents, disableCanvasPointerEvents]
+  );
 };

-export const DialogContent = forwardRef(
+const ContentContainer = forwardRef(
  (
    {
      children,
@ -273,36 +302,53 @@ export const DialogContent = forwardRef(
      minHeight,
      ...props
    }: ComponentProps<typeof Primitive.Content> &
-      UseDraggableProps & {
+      Partial<UseDraggableProps> & {
        css?: CSS;
      },
    forwardedRef: Ref<HTMLDivElement>
  ) => {
-    const { resize } = useContext(DialogContext);
-    const { ref: draggableRef, ...draggableProps } = useDraggable({
+    const { resize, isMaximized } = useContext(DialogContext);
+    const { ref, ...draggableProps } = useDraggable({
      width,
      height,
      x,
      y,
      minWidth,
      minHeight,
+      isMaximized,
+    });
+    const setPointerEvents = useSetPointerEvents(ref);
+
+    const [_, setElement] = useResize({
+      onResizeStart: setPointerEvents?.("none"),
+      onResizeEnd: setPointerEvents?.("auto"),
    });

-    const { ref: pointerEventsRef, ...pointerEventsProps } =
-      useSetPointerEvents();
+    return (
+      <Primitive.Content
+        className={contentStyle({ className, css, resize })}
+        onDragStartCapture={setPointerEvents("none")}
+        onDragEndCapture={setPointerEvents("auto")}
+        {...draggableProps}
+        {...props}
+        ref={mergeRefs(forwardedRef, ref, setElement)}
+      >
+        {children}
+      </Primitive.Content>
+    );
+  }
+);
+ContentContainer.displayName = "ContentContainer";

+export const DialogContent = forwardRef(
+  (
+    props: ComponentProps<typeof ContentContainer>,
+    forwardedRef: Ref<HTMLDivElement>
+  ) => {
    return (
      <Primitive.Portal>
        <Primitive.Overlay className={overlayStyle()} />
-        <Primitive.Content
-          className={contentStyle({ className, css, resize })}
-          {...draggableProps}
-          {...pointerEventsProps}
-          {...props}
-          ref={mergeRefs(forwardedRef, draggableRef, pointerEventsRef)}
-        >
-          {children}
-        </Primitive.Content>
+        <ContentContainer {...props} ref={forwardedRef} />
      </Primitive.Portal>
    );
  }
@ -378,12 +424,6 @@ const overlayStyle = css({
  inset: 0,
 });

-const centeredContent: CSSProperties = {
-  top: "50%",
-  left: "50%",
-  transform: "translate(-50%, -50%)",
-};
-
 const contentStyle = css(panelStyle, {
  position: "fixed",
  width: "min-content",
--- a/packages/design-system/src/components/floating-panel.tsx
+++ b/packages/design-system/src/components/floating-panel.tsx
@ -86,8 +86,8 @@ export const FloatingPanel = ({
    null
  );
  const triggerRef = useRef<HTMLButtonElement>(null);
-  const [x, setX] = useState<number>();
-  const [y, setY] = useState<number>();
+  const [position, setPosition] = useState<{ x: number; y: number }>();
+  const positionIsSetRef = useRef(false);

  const calcPosition = useCallback(() => {
    if (
@ -95,7 +95,9 @@ export const FloatingPanel = ({
      containerRef.current === null ||
      contentElement === null ||
      // When centering the dialog, we don't need to calculate the position
-      placement === "center"
+      placement === "center" ||
+      // After we positioned it once, we leave it alone to avoid jumps when user is scrolling the trigger
+      positionIsSetRef.current
    ) {
      return;
    }
@ -125,11 +127,18 @@ export const FloatingPanel = ({
        placement === "bottom" && flip(),
        offset(offsetProp),
      ],
-    }).then(({ x, y }) => {
-      setX(x);
-      setY(y);
+    }).then((position) => {
+      setPosition(position);
+      positionIsSetRef.current = true;
    });
-  }, [contentElement, triggerRef, containerRef, placement, offsetProp]);
+  }, [
+    positionIsSetRef,
+    contentElement,
+    triggerRef,
+    containerRef,
+    placement,
+    offsetProp,
+  ]);

  useLayoutEffect(calcPosition, [calcPosition]);

@ -148,8 +157,7 @@ export const FloatingPanel = ({
        className={contentStyle()}
        width={width}
        height={height}
-        x={x}
-        y={y}
+        {...position}
        onInteractOutside={(event) => {
          // When a dialog is centered, we don't want to close it when clicking outside
          // This allows having inline and left positioned dialogs open at the same time as a centered dialog,
--- a/packages/design-system/src/utilities.ts
+++ b/packages/design-system/src/utilities.ts
@ -94,9 +94,9 @@ export const useResize = ({
  onResizeEnd,
  timeout = 300,
 }: {
-  onResizeStart?: () => void;
-  onResize?: () => void;
-  onResizeEnd?: () => void;
+  onResizeStart?: (entries: Array<ResizeObserverEntry>) => void;
+  onResize?: (entries: Array<ResizeObserverEntry>) => void;
+  onResizeEnd?: (entries: Array<ResizeObserverEntry>) => void;
  timeout?: number;
 }) => {
  const [element, ref] = useState<HTMLElement | null>(null);
@ -115,7 +115,7 @@ export const useResize = ({
    }
    // Mark resizing as on a new observer instance, we will use this to skip first resize event.
    isResizingRef.current = undefined;
-    const observer = new ResizeObserver(() => {
+    const observer = new ResizeObserver((entries) => {
      // Resize observer called first time is not a start of resize
      if (isResizingRef.current === undefined) {
        isResizingRef.current = false;
@ -123,12 +123,12 @@ export const useResize = ({
      }
      if (isResizingRef.current === false) {
        isResizingRef.current = true;
-        onResizeStartRef.current?.();
+        onResizeStartRef.current?.(entries);
      }
-      onResizeRef.current?.();
+      onResizeRef.current?.(entries);
      clearTimeout(timeoutRef.current);
      timeoutRef.current = setTimeout(() => {
-        onResizeEndRef.current?.();
+        onResizeEndRef.current?.(entries);
        isResizingRef.current = false;
      }, timeout);
    });