From 7617ff1babac2ea553272c76a755dbcde7c84111 Mon Sep 17 00:00:00 2001 From: Balazs Bajorics <2226774+balazsbajorics@users.noreply.github.com> Date: Wed, 16 Oct 2024 10:17:08 +0200 Subject: [PATCH] Fix: bottom strategy controls go under selection outline (#6547) **Problem:** ![image](https://github.com/user-attachments/assets/32e2698d-3d12-4e98-a37c-5ec411a3408c) Problem: The selection outline lives below the grid cell outline. Desired behavior:the grid cell outline should be among the bottommost controls/decorators we draw. **Fix:** The strategy controls already have a setting whether they want to be bottom, undefined or top. The fix is to actually sort them into three arrays by their position, and then render the bottom batch under any other control in NewCanvasControls. Mid and Top remain where they were. **Commit Details:** (< vv pls delete this section if's not relevant) - Added helper pushUniquelyBy - useGetApplicableStrategyControls exhaustively collects bottom, middle and top controls - NewCanvasControls is updated --- .../canvas-strategies/canvas-strategies.tsx | 52 +++++++++++++------ .../canvas/controls/new-canvas-controls.tsx | 32 ++++++++++-- ...performance-regression-tests.spec.tsx.snap | 10 ++++ .../performance-regression-tests.spec.tsx | 8 +-- editor/src/core/shared/array-utils.ts | 6 +++ 5 files changed, 84 insertions(+), 24 deletions(-) diff --git a/editor/src/components/canvas/canvas-strategies/canvas-strategies.tsx b/editor/src/components/canvas/canvas-strategies/canvas-strategies.tsx index 97f5895519ff..f95c64cacb59 100644 --- a/editor/src/components/canvas/canvas-strategies/canvas-strategies.tsx +++ b/editor/src/components/canvas/canvas-strategies/canvas-strategies.tsx @@ -1,6 +1,6 @@ import React from 'react' import { createSelector } from 'reselect' -import { addAllUniquelyBy, mapDropNulls, sortBy } from '../../../core/shared/array-utils' +import { mapDropNulls, pushUniquelyBy, sortBy } from '../../../core/shared/array-utils' import type { ElementInstanceMetadataMap } from '../../../core/shared/element-template' import { arrayEqualsByReference, assertNever } from '../../../core/shared/utils' import type { @@ -648,9 +648,15 @@ function controlPriorityToNumber(prio: ControlWithProps['priority']): numbe } } -export function useGetApplicableStrategyControls( - localSelectedViews: Array, -): Array> { +const controlEquals = (l: ControlWithProps, r: ControlWithProps) => { + return l.control === r.control && l.key === r.key +} + +export function useGetApplicableStrategyControls(localSelectedViews: Array): { + bottomStrategyControls: Array> + middleStrategyControls: Array> + topStrategyControls: Array> +} { const applicableStrategies = useGetApplicableStrategies(localSelectedViews) const currentStrategy = useDelayedCurrentStrategy() const currentlyInProgress = useEditorState( @@ -661,30 +667,44 @@ export function useGetApplicableStrategyControls( 'useGetApplicableStrategyControls currentlyInProgress', ) return React.useMemo(() => { - let applicableControls: Array> = [] let isResizable: boolean = false + const bottomStrategyControls: Array> = [] + const middleStrategyControls: Array> = [] + const topStrategyControls: Array> = [] // Add the controls for currently applicable strategies. for (const strategy of applicableStrategies) { if (isResizableStrategy(strategy)) { isResizable = true } const strategyControls = getApplicableControls(currentStrategy, strategy) - applicableControls = addAllUniquelyBy( - applicableControls, - strategyControls, - (l, r) => l.control === r.control && l.key === r.key, - ) + + // uniquely add the strategyControls to the bottom, middle, and top arrays + for (const control of strategyControls) { + switch (control.priority) { + case 'bottom': + pushUniquelyBy(bottomStrategyControls, control, controlEquals) + break + case undefined: + pushUniquelyBy(middleStrategyControls, control, controlEquals) + break + case 'top': + pushUniquelyBy(topStrategyControls, control, controlEquals) + break + default: + assertNever(control.priority) + } + } } // Special case controls. if (!isResizable && !currentlyInProgress) { - applicableControls.push(notResizableControls) + middleStrategyControls.push(notResizableControls) } - applicableControls = applicableControls.sort( - (a, b) => controlPriorityToNumber(a.priority) - controlPriorityToNumber(b.priority), - ) - - return applicableControls + return { + bottomStrategyControls: bottomStrategyControls, + middleStrategyControls: middleStrategyControls, + topStrategyControls: topStrategyControls, + } }, [applicableStrategies, currentStrategy, currentlyInProgress]) } diff --git a/editor/src/components/canvas/controls/new-canvas-controls.tsx b/editor/src/components/canvas/controls/new-canvas-controls.tsx index 344d57ed840d..bd1863cff4c7 100644 --- a/editor/src/components/canvas/controls/new-canvas-controls.tsx +++ b/editor/src/components/canvas/controls/new-canvas-controls.tsx @@ -285,7 +285,8 @@ const NewCanvasControlsInner = (props: NewCanvasControlsInnerProps) => { const dispatch = useDispatch() const colorTheme = useColorTheme() - const strategyControls = useGetApplicableStrategyControls(localSelectedViews) + const { bottomStrategyControls, middleStrategyControls, topStrategyControls } = + useGetApplicableStrategyControls(localSelectedViews) const [inspectorHoveredControls] = useAtom(InspectorHoveredCanvasControls) const [inspectorFocusedControls] = useAtom(InspectorFocusedCanvasControls) @@ -523,6 +524,11 @@ const NewCanvasControlsInner = (props: NewCanvasControlsInnerProps) => { const resizeStatus = getResizeStatus() + const showStrategyControls = + isMyProject && + isSelectOrInsertMode(editorMode) && + !EP.multiplePathsAllWithTheSameUID(localSelectedViews) + return ( <>
{ {when( resizeStatus !== 'disabled', <> + {when( + showStrategyControls, + <> + {bottomStrategyControls.map((c) => ( + + ))} + , + )} {renderHighlightControls()} {unless(dragInteractionActive, )} {unless( @@ -595,14 +613,20 @@ const NewCanvasControlsInner = (props: NewCanvasControlsInnerProps) => { {renderTextEditableControls()} {when(isSelectMode(editorMode), )} {when( - isSelectOrInsertMode(editorMode) && - !EP.multiplePathsAllWithTheSameUID(localSelectedViews), + showStrategyControls, <> {when( isSelectMode(editorMode) || isInsertMode(editorMode), , )} - {strategyControls.map((c) => ( + {middleStrategyControls.map((c) => ( + + ))} + {topStrategyControls.map((c) => ( { const renderCountAfter = renderResult.getNumberOfRenders() // if this breaks, GREAT NEWS but update the test please :) - expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`864`) + expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`868`) expect(renderResult.getRenderInfo()).toMatchSnapshot() }) @@ -127,7 +127,7 @@ describe('React Render Count Tests -', () => { const renderCountAfter = renderResult.getNumberOfRenders() // if this breaks, GREAT NEWS but update the test please :) - expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`1116`) + expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`1120`) expect(renderResult.getRenderInfo()).toMatchSnapshot() }) @@ -183,7 +183,7 @@ describe('React Render Count Tests -', () => { const renderCountAfter = renderResult.getNumberOfRenders() // if this breaks, GREAT NEWS but update the test please :) - expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`653`) + expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`654`) expect(renderResult.getRenderInfo()).toMatchSnapshot() }) @@ -249,7 +249,7 @@ describe('React Render Count Tests -', () => { const renderCountAfter = renderResult.getNumberOfRenders() // if this breaks, GREAT NEWS but update the test please :) - expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`778`) + expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`779`) expect(renderResult.getRenderInfo()).toMatchSnapshot() }) }) diff --git a/editor/src/core/shared/array-utils.ts b/editor/src/core/shared/array-utils.ts index ebaadea6b50b..8549b2d15c1b 100644 --- a/editor/src/core/shared/array-utils.ts +++ b/editor/src/core/shared/array-utils.ts @@ -272,6 +272,12 @@ export function addUniquely(array: Array, value: T, eq: (l: T, r: T) => boolean): void { + if (array.findIndex((a) => eq(a, value)) === -1) { + array.push(value) + } +} + export function addAllUniquely( array: Array, values: Array,