Skip to content

Commit

Permalink
Fix: bottom strategy controls go under selection outline (#6547)
Browse files Browse the repository at this point in the history
**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
  • Loading branch information
balazsbajorics authored Oct 16, 2024
1 parent 9c831a1 commit 7617ff1
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 24 deletions.
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -648,9 +648,15 @@ function controlPriorityToNumber(prio: ControlWithProps<any>['priority']): numbe
}
}

export function useGetApplicableStrategyControls(
localSelectedViews: Array<ElementPath>,
): Array<ControlWithProps<unknown>> {
const controlEquals = (l: ControlWithProps<any>, r: ControlWithProps<any>) => {
return l.control === r.control && l.key === r.key
}

export function useGetApplicableStrategyControls(localSelectedViews: Array<ElementPath>): {
bottomStrategyControls: Array<ControlWithProps<unknown>>
middleStrategyControls: Array<ControlWithProps<unknown>>
topStrategyControls: Array<ControlWithProps<unknown>>
} {
const applicableStrategies = useGetApplicableStrategies(localSelectedViews)
const currentStrategy = useDelayedCurrentStrategy()
const currentlyInProgress = useEditorState(
Expand All @@ -661,30 +667,44 @@ export function useGetApplicableStrategyControls(
'useGetApplicableStrategyControls currentlyInProgress',
)
return React.useMemo(() => {
let applicableControls: Array<ControlWithProps<unknown>> = []
let isResizable: boolean = false
const bottomStrategyControls: Array<ControlWithProps<unknown>> = []
const middleStrategyControls: Array<ControlWithProps<unknown>> = []
const topStrategyControls: Array<ControlWithProps<unknown>> = []
// 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])
}

Expand Down
32 changes: 28 additions & 4 deletions editor/src/components/canvas/controls/new-canvas-controls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -523,6 +524,11 @@ const NewCanvasControlsInner = (props: NewCanvasControlsInnerProps) => {

const resizeStatus = getResizeStatus()

const showStrategyControls =
isMyProject &&
isSelectOrInsertMode(editorMode) &&
!EP.multiplePathsAllWithTheSameUID(localSelectedViews)

return (
<>
<div
Expand Down Expand Up @@ -557,6 +563,18 @@ const NewCanvasControlsInner = (props: NewCanvasControlsInnerProps) => {
{when(
resizeStatus !== 'disabled',
<>
{when(
showStrategyControls,
<>
{bottomStrategyControls.map((c) => (
<RenderControlMemoized
key={c.key}
control={c.control.control}
propsForControl={c.props}
/>
))}
</>,
)}
{renderHighlightControls()}
{unless(dragInteractionActive, <LayoutParentControl />)}
{unless(
Expand Down Expand Up @@ -595,14 +613,20 @@ const NewCanvasControlsInner = (props: NewCanvasControlsInnerProps) => {
{renderTextEditableControls()}
{when(isSelectMode(editorMode), <AbsoluteChildrenOutline />)}
{when(
isSelectOrInsertMode(editorMode) &&
!EP.multiplePathsAllWithTheSameUID(localSelectedViews),
showStrategyControls,
<>
{when(
isSelectMode(editorMode) || isInsertMode(editorMode),
<GridMeasurementHelpers />,
)}
{strategyControls.map((c) => (
{middleStrategyControls.map((c) => (
<RenderControlMemoized
key={c.key}
control={c.control.control}
propsForControl={c.props}
/>
))}
{topStrategyControls.map((c) => (
<RenderControlMemoized
key={c.key}
control={c.control.control}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Array [
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)(DataReferenceParentOutline)",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
Expand Down Expand Up @@ -808,6 +809,7 @@ Array [
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)(DataReferenceParentOutline)",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
Expand Down Expand Up @@ -1480,6 +1482,7 @@ Array [
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)(DataReferenceParentOutline)",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
Expand Down Expand Up @@ -1770,6 +1773,7 @@ Array [
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)(DataReferenceParentOutline)",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
Expand Down Expand Up @@ -2374,6 +2378,7 @@ Array [
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)(DataReferenceParentOutline)",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
Expand Down Expand Up @@ -2482,6 +2487,7 @@ Array [
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)(DataReferenceParentOutline)",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
Expand Down Expand Up @@ -2601,6 +2607,7 @@ Array [
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)(DataReferenceParentOutline)",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
Expand Down Expand Up @@ -2879,6 +2886,7 @@ Array [
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)(DataReferenceParentOutline)",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
Expand Down Expand Up @@ -3359,6 +3367,7 @@ Array [
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)(DataReferenceParentOutline)",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
Expand Down Expand Up @@ -3409,6 +3418,7 @@ Array [
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)(DataReferenceParentOutline)",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
"/div/div/UtopiaSpiedFunctionComponent(NewCanvasControlsInner)/Symbol(react.memo)()",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ describe('React Render Count Tests -', () => {

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()
})

Expand Down Expand Up @@ -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()
})

Expand Down Expand Up @@ -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()
})

Expand Down Expand Up @@ -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()
})
})
6 changes: 6 additions & 0 deletions editor/src/core/shared/array-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,12 @@ export function addUniquely<T extends string | number | boolean | null | undefin
}
}

export function pushUniquelyBy<T>(array: Array<T>, value: T, eq: (l: T, r: T) => boolean): void {
if (array.findIndex((a) => eq(a, value)) === -1) {
array.push(value)
}
}

export function addAllUniquely<T extends string | number | boolean | null | undefined>(
array: Array<T>,
values: Array<T>,
Expand Down

0 comments on commit 7617ff1

Please sign in to comment.