From c187059c8f82dfa429fc7d339f7785875e463213 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Thu, 15 Aug 2024 14:53:13 -0700 Subject: [PATCH] Fix flicker when dragging a node through a node gap (#231) This simplifies the rect calculations by making the gap between pixels into a padding applied directly to the leaf nodes. This means the gaps won't be present in the overlay layer, so when dragging a node around it is always over an OverlayNode and the pendingAction won't be unset. Also simplifies onDrop handling --- frontend/layout/lib/TileLayout.tsx | 26 ++++----- frontend/layout/lib/layoutModel.ts | 85 ++++++++++++++++++----------- frontend/layout/lib/tilelayout.less | 2 +- 3 files changed, 66 insertions(+), 47 deletions(-) diff --git a/frontend/layout/lib/TileLayout.tsx b/frontend/layout/lib/TileLayout.tsx index b4bc200a3..3d0c00c4b 100644 --- a/frontend/layout/lib/TileLayout.tsx +++ b/frontend/layout/lib/TileLayout.tsx @@ -236,7 +236,7 @@ const DisplayNode = ({ layoutModel, layoutNode, contents }: DisplayNodeProps) => layoutNode.data, ]); - // Register the tile item as a draggable component + // Register the display node as a draggable item useEffect(() => { drag(dragHandleRef); }, [drag, dragHandleRef.current]); @@ -320,7 +320,6 @@ const OverlayNode = ({ layoutNode, layoutModel }: OverlayNodeProps) => { const additionalProps = useLayoutNode(layoutModel, layoutNode); const overlayRef = useRef(null); const generation = useAtomValue(layoutModel.generationAtom); - const pendingAction = useAtomValue(layoutModel.pendingAction.throttledValueAtom); const [, drop] = useDrop( () => ({ @@ -333,27 +332,24 @@ const OverlayNode = ({ layoutNode, layoutModel }: OverlayNodeProps) => { return false; }, drop: (_, monitor) => { - // console.log("drop start", layoutNode.id, layoutTreeState.pendingAction); - if (!monitor.didDrop() && pendingAction) { - layoutModel.treeReducer({ - type: LayoutTreeActionType.CommitPendingAction, - }); + if (!monitor.didDrop()) { + layoutModel.onDrop(); } }, - hover: throttle(30, (_, monitor: DropTargetMonitor) => { + hover: throttle(50, (_, monitor: DropTargetMonitor) => { if (monitor.isOver({ shallow: true })) { - if (monitor.canDrop() && layoutModel.displayContainerRef?.current) { + if (monitor.canDrop() && layoutModel.displayContainerRef?.current && additionalProps?.rect) { const dragItem = monitor.getItem(); // console.log("computing operation", layoutNode, dragItem, additionalProps.rect); const offset = monitor.getClientOffset(); - const containerRect = layoutModel.displayContainerRef.current?.getBoundingClientRect(); - offset.x -= containerRect?.x; - offset.y -= containerRect?.y; + const containerRect = layoutModel.displayContainerRef.current.getBoundingClientRect(); + offset.x -= containerRect.x; + offset.y -= containerRect.y; layoutModel.treeReducer({ type: LayoutTreeActionType.ComputeMove, node: layoutNode, nodeToMove: dragItem, - direction: determineDropDirection(additionalProps?.rect, offset), + direction: determineDropDirection(additionalProps.rect, offset), } as LayoutTreeComputeMoveNodeAction); } else { layoutModel.treeReducer({ @@ -363,10 +359,10 @@ const OverlayNode = ({ layoutNode, layoutModel }: OverlayNodeProps) => { } }), }), - [layoutNode, pendingAction, generation, additionalProps, layoutModel.displayContainerRef] + [layoutNode, generation, additionalProps, layoutModel.displayContainerRef] ); - // Register the tile item as a draggable component + // Register the overlay node as a drop target useEffect(() => { drop(overlayRef); }, []); diff --git a/frontend/layout/lib/layoutModel.ts b/frontend/layout/lib/layoutModel.ts index a35e8619c..5a4257a11 100644 --- a/frontend/layout/lib/layoutModel.ts +++ b/frontend/layout/lib/layoutModel.ts @@ -131,6 +131,18 @@ export class LayoutModel { */ lastMagnifiedNodeId: string; + /** + * The size of the resize handles, in CSS pixels. + * The resize handle size is double the gap size, or double the default gap size, whichever is greater. + * @see gapSizePx @see DefaultGapSizePx + */ + private resizeHandleSizePx: number; + /** + * Half of the size of the resize handles, in CSS pixels. + * + * @see resizeHandleSizePx This is just a precomputed halving of the resize handle size. + */ + private halfResizeHandleSizePx: number; /** * A context used by the resize handles to keep track of precomputed values for the current resize operation. */ @@ -165,6 +177,8 @@ export class LayoutModel { this.renderPreview = renderPreview; this.onNodeDelete = onNodeDelete; this.gapSizePx = gapSizePx ?? DefaultGapSizePx; + this.halfResizeHandleSizePx = this.gapSizePx > 5 ? this.gapSizePx : DefaultGapSizePx; + this.resizeHandleSizePx = 2 * this.halfResizeHandleSizePx; this.leafs = []; this.additionalProps = atom({}); @@ -209,7 +223,7 @@ export class LayoutModel { this.pendingAction = atomWithThrottle(null, 10); this.placeholderTransform = atom((get: Getter) => { const pendingAction = get(this.pendingAction.throttledValueAtom); - console.log("update to pending action", pendingAction); + // console.log("update to pending action", pendingAction); return this.getPlaceholderTransform(pendingAction); }); @@ -232,7 +246,7 @@ export class LayoutModel { * @param action The action to perform. */ treeReducer(action: LayoutTreeAction) { - console.log("treeReducer", action, this); + // console.log("treeReducer", action, this); let stateChanged = false; switch (action.type) { case LayoutTreeActionType.ComputeMove: @@ -399,24 +413,17 @@ export class LayoutModel { const nodeRect: Dimensions = node.id === this.treeState.rootNode.id ? getBoundingRect() : additionalProps.rect; const nodeIsRow = node.flexDirection === FlexDirection.Row; - const nodePixelsMinusGap = - (nodeIsRow ? nodeRect.width : nodeRect.height) - this.gapSizePx * (node.children.length - 1); + const nodePixels = nodeIsRow ? nodeRect.width : nodeRect.height; const totalChildrenSize = node.children.reduce((acc, child) => acc + getNodeSize(child), 0); - const pixelToSizeRatio = totalChildrenSize / nodePixelsMinusGap; + const pixelToSizeRatio = totalChildrenSize / nodePixels; let lastChildRect: Dimensions; const resizeHandles: ResizeHandleProps[] = []; - node.children.forEach((child, i) => { + for (const child of node.children) { const childSize = getNodeSize(child); const rect: Dimensions = { - top: - !nodeIsRow && lastChildRect - ? lastChildRect.top + lastChildRect.height + this.gapSizePx - : nodeRect.top, - left: - nodeIsRow && lastChildRect - ? lastChildRect.left + lastChildRect.width + this.gapSizePx - : nodeRect.left, + top: !nodeIsRow && lastChildRect ? lastChildRect.top + lastChildRect.height : nodeRect.top, + left: nodeIsRow && lastChildRect ? lastChildRect.left + lastChildRect.width : nodeRect.left, width: nodeIsRow ? childSize / pixelToSizeRatio : nodeRect.width, height: nodeIsRow ? nodeRect.height : childSize / pixelToSizeRatio, }; @@ -426,24 +433,32 @@ export class LayoutModel { transform, }; - const resizeHandleDimensions: Dimensions = { - top: nodeIsRow ? rect.top : rect.top + rect.height - 0.5 * this.gapSizePx, - left: nodeIsRow ? rect.left + rect.width - 0.5 * this.gapSizePx : rect.left, - width: nodeIsRow ? 2 * this.gapSizePx : rect.width, - height: nodeIsRow ? rect.height : 2 * this.gapSizePx, - }; - resizeHandles.push({ - id: `${node.id}-${i}`, - parentNodeId: node.id, - parentIndex: i, - transform: setTransform(resizeHandleDimensions, true, false), - flexDirection: node.flexDirection, - centerPx: (nodeIsRow ? resizeHandleDimensions.left : resizeHandleDimensions.top) + this.gapSizePx, - }); + // We only want the resize handles in between nodes, this ensures we have n-1 handles. + if (lastChildRect) { + const resizeHandleIndex = resizeHandles.length; + const resizeHandleDimensions: Dimensions = { + top: nodeIsRow + ? lastChildRect.top + : lastChildRect.top + lastChildRect.height - this.halfResizeHandleSizePx, + left: nodeIsRow + ? lastChildRect.left + lastChildRect.width - this.halfResizeHandleSizePx + : lastChildRect.left, + width: nodeIsRow ? this.resizeHandleSizePx : lastChildRect.width, + height: nodeIsRow ? lastChildRect.height : this.resizeHandleSizePx, + }; + resizeHandles.push({ + id: `${node.id}-${resizeHandleIndex}`, + parentNodeId: node.id, + parentIndex: resizeHandleIndex, + transform: setTransform(resizeHandleDimensions, true, false), + flexDirection: node.flexDirection, + centerPx: + (nodeIsRow ? resizeHandleDimensions.left : resizeHandleDimensions.top) + + this.halfResizeHandleSizePx, + }); + } lastChildRect = rect; - }); - - resizeHandles.pop(); + } additionalPropsMap[node.id] = { ...additionalProps, @@ -575,6 +590,14 @@ export class LayoutModel { await this.onNodeDelete?.(node.data); } + onDrop() { + if (this.getter(this.pendingAction.currentValueAtom)) { + this.treeReducer({ + type: LayoutTreeActionType.CommitPendingAction, + }); + } + } + /** * Callback that is invoked when the TileLayout container is being resized. */ diff --git a/frontend/layout/lib/tilelayout.less b/frontend/layout/lib/tilelayout.less index 29319368f..05c20fad0 100644 --- a/frontend/layout/lib/tilelayout.less +++ b/frontend/layout/lib/tilelayout.less @@ -69,7 +69,6 @@ overflow: hidden; width: 100%; height: 100%; - z-index: inherit; &.dragging { filter: blur(8px); @@ -119,6 +118,7 @@ .tile-leaf { overflow: hidden; + padding: calc(var(--gap-size-px) / 2); } .placeholder {