From 9dc2fa3ce419452ffc4bd5b4d14eebb1ed392710 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bertalan=20K=C3=B6rmendy?= Date: Wed, 2 Oct 2024 10:09:49 +0200 Subject: [PATCH] Specify elements to rerender in grid strategies (#6443) ## Problem The grid strategies opt to rerender all elements in their strategy result, which is adds a performance penalty. ## Fix Only re-render the elements affected by a given strategy --- .../strategies/grid-draw-to-insert-strategy.tsx | 5 +---- .../strategies/grid-rearrange-keyboard-strategy.ts | 5 +---- .../strategies/grid-rearrange-move-strategy.ts | 5 +---- .../strategies/grid-resize-element-strategy.ts | 5 +---- .../strategies/rearrange-grid-swap-strategy.ts | 8 +------- .../canvas-strategies/strategies/resize-grid-strategy.ts | 5 +---- .../strategies/set-grid-gap-strategy.tsx | 5 +---- 7 files changed, 7 insertions(+), 31 deletions(-) diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-draw-to-insert-strategy.tsx b/editor/src/components/canvas/canvas-strategies/strategies/grid-draw-to-insert-strategy.tsx index 50965cc71a8e..8846b941cb08 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/grid-draw-to-insert-strategy.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-draw-to-insert-strategy.tsx @@ -245,10 +245,7 @@ const gridDrawToInsertStrategyInner = : null, ]), ], - // FIXME: This was added as a default value in https://github.com/concrete-utopia/utopia/pull/6408 - // This was to maintain the existing behaviour, but it should be replaced with a more specific value - // appropriate to this particular case. - 'rerender-all-elements', + [targetParent], { strategyGeneratedUidsCache: { [insertionSubject.uid]: maybeWrapperWithUid?.uid, diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-rearrange-keyboard-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/grid-rearrange-keyboard-strategy.ts index 2b34b4cddc36..082370c9d723 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/grid-rearrange-keyboard-strategy.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-rearrange-keyboard-strategy.ts @@ -133,10 +133,7 @@ export function gridRearrangeResizeKeyboardStrategy( gridRowStart, gridRowEnd, }), - // FIXME: This was added as a default value in https://github.com/concrete-utopia/utopia/pull/6408 - // This was to maintain the existing behaviour, but it should be replaced with a more specific value - // appropriate to this particular case. - 'rerender-all-elements', + [parentGridPath], ) }, } diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-rearrange-move-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/grid-rearrange-move-strategy.ts index c9ab491743f1..f3bc35a2a0a9 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/grid-rearrange-move-strategy.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-rearrange-move-strategy.ts @@ -155,10 +155,7 @@ export const gridRearrangeMoveStrategy: CanvasStrategyFactory = ( return strategyApplicationResult( [...midInteractionCommands, ...onCompleteCommands, ...commands], - // FIXME: This was added as a default value in https://github.com/concrete-utopia/utopia/pull/6408 - // This was to maintain the existing behaviour, but it should be replaced with a more specific value - // appropriate to this particular case. - 'rerender-all-elements', + [parentGridPath], patch, ) }, diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-resize-element-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/grid-resize-element-strategy.ts index daa9996bc207..c3324fbacabc 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/grid-resize-element-strategy.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-resize-element-strategy.ts @@ -138,10 +138,7 @@ export const gridResizeElementStrategy: CanvasStrategyFactory = ( return strategyApplicationResult( setGridPropsCommands(selectedElement, gridTemplate, gridPropsWithDragOver(gridProps)), - // FIXME: This was added as a default value in https://github.com/concrete-utopia/utopia/pull/6408 - // This was to maintain the existing behaviour, but it should be replaced with a more specific value - // appropriate to this particular case. - 'rerender-all-elements', + [parentGridPath], { grid: { ...customState.grid, targetCellData: targetCell }, }, diff --git a/editor/src/components/canvas/canvas-strategies/strategies/rearrange-grid-swap-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/rearrange-grid-swap-strategy.ts index ef9bf83e2afe..c11d53c4b6d1 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/rearrange-grid-swap-strategy.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/rearrange-grid-swap-strategy.ts @@ -121,13 +121,7 @@ export const rearrangeGridSwapStrategy: CanvasStrategyFactory = ( return emptyStrategyApplicationResult } - return strategyApplicationResult( - commands, - // FIXME: This was added as a default value in https://github.com/concrete-utopia/utopia/pull/6408 - // This was to maintain the existing behaviour, but it should be replaced with a more specific value - // appropriate to this particular case. - 'rerender-all-elements', - ) + return strategyApplicationResult(commands, [EP.parentPath(selectedElement)]) }, } } diff --git a/editor/src/components/canvas/canvas-strategies/strategies/resize-grid-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/resize-grid-strategy.ts index 6a0d12b74acc..ae80e5b24252 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/resize-grid-strategy.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/resize-grid-strategy.ts @@ -118,10 +118,7 @@ export const resizeGridStrategy: CanvasStrategyFactory = ( } if (!canResizeGridTemplate(originalValues)) { - return strategyApplicationResult( - [setCursorCommand(CSSCursor.NotPermitted)], - 'rerender-all-elements', - ) + return strategyApplicationResult([setCursorCommand(CSSCursor.NotPermitted)], []) } const expandedOriginalValues = expandGridDimensions(originalValues.dimensions) diff --git a/editor/src/components/canvas/canvas-strategies/strategies/set-grid-gap-strategy.tsx b/editor/src/components/canvas/canvas-strategies/strategies/set-grid-gap-strategy.tsx index cb2afe25f6de..9b9b5d2b3ef5 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/set-grid-gap-strategy.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/set-grid-gap-strategy.tsx @@ -173,10 +173,7 @@ export const setGridGapStrategy: CanvasStrategyFactory = ( if (shouldTearOffGapByAxis) { return strategyApplicationResult( [deleteProperties('always', selectedElement, [axisStyleProp])], - // FIXME: This was added as a default value in https://github.com/concrete-utopia/utopia/pull/6408 - // This was to maintain the existing behaviour, but it should be replaced with a more specific value - // appropriate to this particular case. - 'rerender-all-elements', + selectedElements, ) }