From 5fa6c0354f45ce4284c8639c0f49ed3e9f57506c Mon Sep 17 00:00:00 2001 From: Oleg Isonen Date: Thu, 9 Jan 2025 16:29:08 +0000 Subject: [PATCH] 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 --- .../css-value-input/value-editor-dialog.tsx | 2 +- .../src/property-parsers/linear-gradient.ts | 2 +- .../design-system/src/components/dialog.tsx | 214 +++++++++++------- .../src/components/floating-panel.tsx | 26 ++- packages/design-system/src/utilities.ts | 14 +- 5 files changed, 153 insertions(+), 105 deletions(-) diff --git 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 index 7cf4d9f03..f4d940b93 100644 --- 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={ ; 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(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) => { 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 & - UseDraggableProps & { + Partial & { css?: CSS; }, forwardedRef: Ref ) => { - 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 ( + + {children} + + ); + } +); +ContentContainer.displayName = "ContentContainer"; +export const DialogContent = forwardRef( + ( + props: ComponentProps, + forwardedRef: Ref + ) => { return ( - - {children} - + ); } @@ -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", diff --git a/packages/design-system/src/components/floating-panel.tsx b/packages/design-system/src/components/floating-panel.tsx index c8a9edbe4..f9f94d6ac 100644 --- 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(null); - const [x, setX] = useState(); - const [y, setY] = useState(); + 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, diff --git a/packages/design-system/src/utilities.ts b/packages/design-system/src/utilities.ts index 963680e02..26d237fce 100644 --- 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) => void; + onResize?: (entries: Array) => void; + onResizeEnd?: (entries: Array) => void; timeout?: number; }) => { const [element, ref] = useState(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); });