From 2157df85de2e3341d1d9009af02cfe1a1142eb55 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 31 Jul 2024 12:49:38 -0700 Subject: [PATCH] Disable block pointer events during layout drag (#183) This will ensure that the webview cannot capture the pointer events and disrupt the drag functionality. This also fixes an issue where greedy and imprecise bounds calculations could result in thrashing of the layoutState.pendingAction field, which could cause the placeholder to flicker. This also fixes issues where some React Effects that were supposed to be debounced or throttled were being invoked too much. This is because useEffect regenerates the callback when it is run, resulting in the debounce or throttle never taking effect. Moving the throttled or debounced logic to a separate callback solves this. --- frontend/app/block/block.tsx | 24 ++-- frontend/app/tab/tabcontent.tsx | 15 +- frontend/layout/lib/TileLayout.tsx | 220 +++++++++++++++++------------ frontend/layout/lib/model.ts | 1 + 4 files changed, 152 insertions(+), 108 deletions(-) diff --git a/frontend/app/block/block.tsx b/frontend/app/block/block.tsx index 82f64e50..b8d97e30 100644 --- a/frontend/app/block/block.tsx +++ b/frontend/app/block/block.tsx @@ -22,6 +22,7 @@ import * as React from "react"; import "./block.less"; export interface LayoutComponentModel { + disablePointerEvents: boolean; onClose?: () => void; onMagnifyToggle?: () => void; dragHandleRef?: React.RefObject; @@ -268,7 +269,7 @@ const BlockFrame_Default_Component = ({ if (preview) { isFocused = true; } - let style: React.CSSProperties = {}; + const style: React.CSSProperties = {}; if (!isFocused && blockData?.meta?.["frame:bordercolor"]) { style.borderColor = blockData.meta["frame:bordercolor"]; } @@ -286,12 +287,13 @@ const BlockFrame_Default_Component = ({ if (preIconButton) { preIconButtonElem = ; } - let endIconsElem: JSX.Element[] = []; + const endIconsElem: JSX.Element[] = []; if (endIconButtons && endIconButtons.length > 0) { - for (let idx = 0; idx < endIconButtons.length; idx++) { - const button = endIconButtons[idx]; - endIconsElem.push(); - } + endIconsElem.push( + ...endIconButtons.map((button, idx) => ( + + )) + ); } const settingsDecl: HeaderIconButton = { elemtype: "iconbutton", @@ -549,7 +551,7 @@ function makeDefaultViewModel(blockId: string): ViewModel { } const BlockPreview = React.memo(({ blockId, layoutModel }: BlockProps) => { - const [blockData, blockDataLoading] = WOS.useWaveObjectValue(WOS.makeORef("block", blockId)); + const [blockData] = WOS.useWaveObjectValue(WOS.makeORef("block", blockId)); if (!blockData) { return null; } @@ -578,7 +580,7 @@ const BlockFull = React.memo(({ blockId, layoutModel }: BlockProps) => { return winData.activeblockid === blockId; }); }); - let isFocused = jotai.useAtomValue(isFocusedAtom); + const isFocused = jotai.useAtomValue(isFocusedAtom); React.useLayoutEffect(() => { setBlockClicked(isFocused); @@ -651,7 +653,11 @@ const BlockFull = React.memo(({ blockId, layoutModel }: BlockProps) => {
{}} />
-
+
Loading...}>{viewElem} diff --git a/frontend/app/tab/tabcontent.tsx b/frontend/app/tab/tabcontent.tsx index fe8fa13d..b4b48d76 100644 --- a/frontend/app/tab/tabcontent.tsx +++ b/frontend/app/tab/tabcontent.tsx @@ -8,6 +8,7 @@ import * as WOS from "@/store/wos"; import * as React from "react"; import { CenteredDiv } from "@/element/quickelems"; +import { ContentRenderer } from "@/layout/lib/model"; import { TileLayout } from "frontend/layout/index"; import { getLayoutStateAtomForTab } from "frontend/layout/lib/layoutAtom"; import { useAtomValue } from "jotai"; @@ -23,23 +24,25 @@ const TabContent = React.memo(({ tabId }: { tabId: string }) => { const tabData = useAtomValue(tabAtom); const tileLayoutContents = useMemo(() => { - function renderBlock( + const renderBlock: ContentRenderer = ( tabData: TabLayoutData, ready: boolean, + disablePointerEvents: boolean, onMagnifyToggle: () => void, onClose: () => void, dragHandleRef: React.RefObject - ) { + ) => { if (!tabData.blockId || !ready) { return null; } const layoutModel: LayoutComponentModel = { - onClose: onClose, - onMagnifyToggle: onMagnifyToggle, - dragHandleRef: dragHandleRef, + disablePointerEvents, + onClose, + onMagnifyToggle, + dragHandleRef, }; return ; - } + }; function renderPreview(tabData: TabLayoutData) { return ; diff --git a/frontend/layout/lib/TileLayout.tsx b/frontend/layout/lib/TileLayout.tsx index e4ee2c89..49c2d1dd 100644 --- a/frontend/layout/lib/TileLayout.tsx +++ b/frontend/layout/lib/TileLayout.tsx @@ -17,7 +17,7 @@ import React, { useRef, useState, } from "react"; -import { DropTargetMonitor, useDrag, useDragLayer, useDrop } from "react-dnd"; +import { DropTargetMonitor, XYCoord, useDrag, useDragLayer, useDrop } from "react-dnd"; import { debounce, throttle } from "throttle-debounce"; import { useDevicePixelRatio } from "use-device-pixel-ratio"; import { globalLayoutTransformsMap } from "./layoutAtom"; @@ -132,11 +132,9 @@ function TileLayoutComponent({ layoutTreeStateAtom, contents, getCursorPoint dragClientOffset: monitor.getClientOffset(), })); - // Effect to detect when the cursor leaves the TileLayout hit trap so we can remove any placeholders. This cannot be done using pointer capture - // because that conflicts with the DnD layer. - useEffect( - debounce(100, () => { - const cursorPoint = getCursorPoint?.() ?? dragClientOffset; + const checkForCursorBounds = useCallback( + debounce(100, (dragClientOffset: XYCoord) => { + const cursorPoint = dragClientOffset ?? getCursorPoint?.(); if (cursorPoint && displayContainerRef.current) { const displayContainerRect = displayContainerRef.current.getBoundingClientRect(); const normalizedX = cursorPoint.x - displayContainerRect.x; @@ -151,9 +149,13 @@ function TileLayoutComponent({ layoutTreeStateAtom, contents, getCursorPoint } } }), - [dragClientOffset] + [getCursorPoint, displayContainerRef, dispatch] ); + // Effect to detect when the cursor leaves the TileLayout hit trap so we can remove any placeholders. This cannot be done using pointer capture + // because that conflicts with the DnD layer. + useEffect(() => checkForCursorBounds(dragClientOffset), [dragClientOffset]); + /** * Callback to update the transforms on the displayed leafs and move the overlay over the display layer when dragging. */ @@ -279,6 +281,7 @@ function TileLayoutComponent({ layoutTreeStateAtom, contents, getCursorPoint { * Determines whether the leaf nodes are ready to be displayed to the user. */ ready: boolean; + + /** + * Determines if a drag operation is in progress. + */ + activeDrag: boolean; } const DisplayNodesWrapper = memo( @@ -355,6 +363,7 @@ const DisplayNodesWrapper = memo( onLeafMagnifyToggle, layoutLeafTransforms, ready, + activeDrag, }: DisplayNodesWrapperProps) => { if (!layoutLeafTransforms) { return null; @@ -366,6 +375,7 @@ const DisplayNodesWrapper = memo( key={leaf.id} layoutNode={leaf} contents={contents} + activeDrag={activeDrag} transform={layoutLeafTransforms[leaf.id]} onLeafClose={onLeafClose} onLeafMagnifyToggle={onLeafMagnifyToggle} @@ -398,11 +408,17 @@ interface DisplayNodeProps { * @param node The node that is closed. */ onLeafClose: (node: LayoutNode) => void; + /** * Determines whether a leaf's contents should be displayed to the user. */ ready: boolean; + /** + * Determines if a drag operation is in progress. + */ + activeDrag: boolean; + /** * Any class names to add to the component. */ @@ -427,6 +443,7 @@ const DisplayNode = memo( onLeafMagnifyToggle, onLeafClose, ready, + activeDrag, className, }: DisplayNodeProps) => { const tileNodeRef = useRef(null); @@ -508,11 +525,18 @@ const DisplayNode = memo( return ( layoutNode.data && (
- {contents.renderContent(layoutNode.data, ready, onMagnifyToggle, onClose, dragHandleRef)} + {contents.renderContent( + layoutNode.data, + ready, + activeDrag, + onMagnifyToggle, + onClose, + dragHandleRef + )}
) ); - }, [layoutNode.data, ready, onClose]); + }, [layoutNode.data, ready, activeDrag, onClose]); return (
{ /** * An overlay to preview pending actions on the layout tree. */ -const Placeholder = ({ layoutTreeState, overlayContainerRef, nodeRefsAtom, style }: PlaceholderProps) => { +const Placeholder = memo(({ layoutTreeState, overlayContainerRef, nodeRefsAtom, style }: PlaceholderProps) => { const [placeholderOverlay, setPlaceholderOverlay] = useState(null); const nodeRefs = useAtomValue(nodeRefsAtom); - useEffect(() => { - let newPlaceholderOverlay: ReactNode; - if (overlayContainerRef?.current) { - switch (layoutTreeState?.pendingAction?.type) { - case LayoutTreeActionType.Move: { - const action = layoutTreeState.pendingAction as LayoutTreeMoveNodeAction; - let parentId: string; - if (action.insertAtRoot) { - parentId = layoutTreeState.rootNode.id; - } else { - parentId = action.parentId; - } - - const parentNode = findNode(layoutTreeState.rootNode, parentId); - if (action.index !== undefined && parentNode) { - const targetIndex = Math.min( - parentNode.children ? parentNode.children.length - 1 : 0, - Math.max(0, action.index - 1) - ); - let targetNode = parentNode?.children?.at(targetIndex); - let targetRef: React.RefObject; - if (targetNode) { - targetRef = nodeRefs.get(targetNode.id); + const updatePlaceholder = useCallback( + throttle(10, (pendingAction: LayoutTreeAction) => { + let newPlaceholderOverlay: ReactNode; + if (overlayContainerRef?.current) { + switch (pendingAction?.type) { + case LayoutTreeActionType.Move: { + const action = pendingAction as LayoutTreeMoveNodeAction; + let parentId: string; + if (action.insertAtRoot) { + parentId = layoutTreeState.rootNode.id; } else { - targetRef = nodeRefs.get(parentNode.id); - targetNode = parentNode; + parentId = action.parentId; } + + const parentNode = findNode(layoutTreeState.rootNode, parentId); + if (action.index !== undefined && parentNode) { + const targetIndex = Math.min( + parentNode.children ? parentNode.children.length - 1 : 0, + Math.max(0, action.index - 1) + ); + let targetNode = parentNode?.children?.at(targetIndex); + let targetRef: React.RefObject; + if (targetNode) { + targetRef = nodeRefs.get(targetNode.id); + } else { + targetRef = nodeRefs.get(parentNode.id); + targetNode = parentNode; + } + if (targetRef?.current) { + const overlayBoundingRect = overlayContainerRef.current.getBoundingClientRect(); + const targetBoundingRect = targetRef.current.getBoundingClientRect(); + + // Placeholder should be either half the height or half the width of the targetNode, depending on the flex direction of the targetNode's parent. + // Default to placing the placeholder in the first half of the target node. + const placeholderDimensions: Dimensions = { + height: + parentNode.flexDirection === FlexDirection.Column + ? targetBoundingRect.height / 2 + : targetBoundingRect.height, + width: + parentNode.flexDirection === FlexDirection.Row + ? targetBoundingRect.width / 2 + : targetBoundingRect.width, + top: targetBoundingRect.top - overlayBoundingRect.top, + left: targetBoundingRect.left - overlayBoundingRect.left, + }; + + if (action.index > targetIndex) { + if (action.index >= (parentNode.children?.length ?? 1)) { + // If there are no more nodes after the specified index, place the placeholder in the second half of the target node (either right or bottom). + placeholderDimensions.top += + parentNode.flexDirection === FlexDirection.Column && + targetBoundingRect.height / 2; + placeholderDimensions.left += + parentNode.flexDirection === FlexDirection.Row && + targetBoundingRect.width / 2; + } else { + // Otherwise, place the placeholder between the target node (the one after which it will be inserted) and the next node + placeholderDimensions.top += + parentNode.flexDirection === FlexDirection.Column && + (3 * targetBoundingRect.height) / 4; + placeholderDimensions.left += + parentNode.flexDirection === FlexDirection.Row && + (3 * targetBoundingRect.width) / 4; + } + } + + const placeholderTransform = setTransform(placeholderDimensions); + newPlaceholderOverlay = ( +
+ ); + } + } + break; + } + case LayoutTreeActionType.Swap: { + const action = pendingAction as LayoutTreeSwapNodeAction; + // console.log("placeholder for swap", action); + const targetNodeId = action.node1Id; + const targetRef = nodeRefs.get(targetNodeId); if (targetRef?.current) { const overlayBoundingRect = overlayContainerRef.current.getBoundingClientRect(); const targetBoundingRect = targetRef.current.getBoundingClientRect(); - - // Placeholder should be either half the height or half the width of the targetNode, depending on the flex direction of the targetNode's parent. - // Default to placing the placeholder in the first half of the target node. const placeholderDimensions: Dimensions = { - height: - parentNode.flexDirection === FlexDirection.Column - ? targetBoundingRect.height / 2 - : targetBoundingRect.height, - width: - parentNode.flexDirection === FlexDirection.Row - ? targetBoundingRect.width / 2 - : targetBoundingRect.width, top: targetBoundingRect.top - overlayBoundingRect.top, left: targetBoundingRect.left - overlayBoundingRect.left, + height: targetBoundingRect.height, + width: targetBoundingRect.width, }; - if (action.index > targetIndex) { - if (action.index >= (parentNode.children?.length ?? 1)) { - // If there are no more nodes after the specified index, place the placeholder in the second half of the target node (either right or bottom). - placeholderDimensions.top += - parentNode.flexDirection === FlexDirection.Column && - targetBoundingRect.height / 2; - placeholderDimensions.left += - parentNode.flexDirection === FlexDirection.Row && targetBoundingRect.width / 2; - } else { - // Otherwise, place the placeholder between the target node (the one after which it will be inserted) and the next node - placeholderDimensions.top += - parentNode.flexDirection === FlexDirection.Column && - (3 * targetBoundingRect.height) / 4; - placeholderDimensions.left += - parentNode.flexDirection === FlexDirection.Row && - (3 * targetBoundingRect.width) / 4; - } - } - const placeholderTransform = setTransform(placeholderDimensions); newPlaceholderOverlay =
; } + break; } - break; + default: + // No-op + break; } - case LayoutTreeActionType.Swap: { - const action = layoutTreeState.pendingAction as LayoutTreeSwapNodeAction; - // console.log("placeholder for swap", action); - const targetNodeId = action.node1Id; - const targetRef = nodeRefs.get(targetNodeId); - if (targetRef?.current) { - const overlayBoundingRect = overlayContainerRef.current.getBoundingClientRect(); - const targetBoundingRect = targetRef.current.getBoundingClientRect(); - const placeholderDimensions: Dimensions = { - top: targetBoundingRect.top - overlayBoundingRect.top, - left: targetBoundingRect.left - overlayBoundingRect.left, - height: targetBoundingRect.height, - width: targetBoundingRect.width, - }; - - const placeholderTransform = setTransform(placeholderDimensions); - newPlaceholderOverlay =
; - } - break; - } - default: - // No-op - break; } - } - setPlaceholderOverlay(newPlaceholderOverlay); - }, [layoutTreeState.pendingAction, nodeRefs, overlayContainerRef]); + setPlaceholderOverlay(newPlaceholderOverlay); + }), + [nodeRefs, overlayContainerRef, layoutTreeState.rootNode] + ); + + useEffect(() => { + updatePlaceholder(layoutTreeState.pendingAction); + }, [layoutTreeState.pendingAction, updatePlaceholder]); return (
{placeholderOverlay}
); -}; +}); diff --git a/frontend/layout/lib/model.ts b/frontend/layout/lib/model.ts index 7642441e..a029d8ef 100644 --- a/frontend/layout/lib/model.ts +++ b/frontend/layout/lib/model.ts @@ -214,6 +214,7 @@ export type WritableLayoutTreeStateAtom = WritableAtom, [v export type ContentRenderer = ( data: T, ready: boolean, + disablePointerEvents: boolean, onMagnifyToggle: () => void, onClose: () => void, dragHandleRef: React.RefObject