From 58a70921f10bae37bf8a8884329e7bbfc768f386 Mon Sep 17 00:00:00 2001
From: Federico Ruggi <1081051+ruggi@users.noreply.github.com>
Date: Thu, 13 Jun 2024 16:36:52 +0200
Subject: [PATCH] Consistent border radii for navigator selection (#5928)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
**Problem:**
Selected rows in the navigator have inconsistent / potato border radii
applied to pretty much any selected row. Moreover, the children
selection also does not have any border radius.
**Fix:**
Contiguous regions of selection now share just 4 rounded corners. This
is done with a hook which is used on both the regular and the condensed
navigator rows, so they share all the underlying logic to figure out
which element should have `borderRadius` applied or not.
Note: there's a reasonable chance there will be corner (pun intended)
cases here and there, but it feels decently solid.
**Manual Tests:**
I hereby swear that:
- [x] I opened a hydrogen project and it loaded
- [x] I could navigate to various routes in Preview mode
Fixes #5927
---
.../navigator-condensed-entry.tsx | 25 ++++-
.../navigator-item/navigator-item.tsx | 105 ++++++++++--------
...e-navigator-selection-bounds-for-entry.tsx | 77 +++++++++++++
3 files changed, 156 insertions(+), 51 deletions(-)
create mode 100644 editor/src/components/navigator/navigator-item/use-navigator-selection-bounds-for-entry.tsx
diff --git a/editor/src/components/navigator/navigator-item/navigator-condensed-entry.tsx b/editor/src/components/navigator/navigator-item/navigator-condensed-entry.tsx
index f298ea20aded..9ea48a1b6002 100644
--- a/editor/src/components/navigator/navigator-item/navigator-condensed-entry.tsx
+++ b/editor/src/components/navigator/navigator-item/navigator-condensed-entry.tsx
@@ -7,7 +7,11 @@ import { Substores, useEditorState } from '../../editor/store/store-hook'
import { condensedNavigatorRow, type CondensedNavigatorRow } from '../navigator-row'
import { MetadataUtils } from '../../../core/model/element-metadata-utils'
import { getNavigatorEntryLabel, labelSelector } from './navigator-item-wrapper'
-import { BasePaddingUnit, elementWarningsSelector } from './navigator-item'
+import {
+ BasePaddingUnit,
+ NavigatorRowBorderRadius,
+ elementWarningsSelector,
+} from './navigator-item'
import { setHighlightedViews, toggleCollapse } from '../../editor/actions/action-creators'
import type { ElementPath } from 'utopia-shared/src/types'
import { unless, when } from '../../../utils/react-conditionals'
@@ -18,6 +22,7 @@ import {
NavigatorRowClickableWrapper,
useGetNavigatorClickActions,
} from './navigator-item-clickable-wrapper'
+import { useNavigatorSelectionBoundsForEntry } from './use-navigator-selection-bounds-for-entry'
function useEntryLabel(entry: NavigatorEntry) {
const labelForTheElement = useEditorState(
@@ -87,6 +92,12 @@ export const CondensedEntryItemWrapper = React.memo(
)
}, [selectedViews, props.navigatorRow])
+ const { isTopOfSelection, isBottomOfSelection } = useNavigatorSelectionBoundsForEntry(
+ props.navigatorRow.entries[0],
+ rowRootSelected,
+ 0,
+ )
+
return (
diff --git a/editor/src/components/navigator/navigator-item/navigator-item.tsx b/editor/src/components/navigator/navigator-item/navigator-item.tsx
index ba2a5105843a..8dba98288451 100644
--- a/editor/src/components/navigator/navigator-item/navigator-item.tsx
+++ b/editor/src/components/navigator/navigator-item/navigator-item.tsx
@@ -68,6 +68,7 @@ import { DataReferenceCartoucheControl } from '../../inspector/sections/componen
import { MapListSourceCartoucheNavigator } from '../../inspector/sections/layout-section/list-source-cartouche'
import { regularNavigatorRow } from '../navigator-row'
import { NavigatorRowClickableWrapper } from './navigator-item-clickable-wrapper'
+import { useNavigatorSelectionBoundsForEntry } from './use-navigator-selection-bounds-for-entry'
export function getItemHeight(navigatorEntry: NavigatorEntry): number {
if (isConditionalClauseNavigatorEntry(navigatorEntry)) {
@@ -168,22 +169,38 @@ const getColors = (
}
}
-const computeResultingStyle = (
- selected: boolean,
- emphasis: Emphasis,
- isInsideComponent: boolean,
- isDynamic: boolean,
- isProbablyScene: boolean,
- fullyVisible: boolean,
- isFocusedComponent: boolean,
- isInFocusedComponentSubtree: boolean,
- isFocusableComponent: boolean,
- isHighlightedForInteraction: boolean,
- isDescendantOfSelected: boolean,
- isErroredGroup: boolean,
- colorTheme: ThemeObject,
- isSingleItem: boolean,
-) => {
+export const NavigatorRowBorderRadius = 5
+
+const computeResultingStyle = (params: {
+ selected: boolean
+ isTopOfSelection: boolean
+ isBottomOfSelection: boolean
+ emphasis: Emphasis
+ isInsideComponent: boolean
+ isProbablyScene: boolean
+ fullyVisible: boolean
+ isFocusedComponent: boolean
+ isFocusableComponent: boolean
+ isHighlightedForInteraction: boolean
+ isDescendantOfSelected: boolean
+ isErroredGroup: boolean
+ colorTheme: ThemeObject
+}) => {
+ const {
+ selected,
+ isTopOfSelection,
+ isBottomOfSelection,
+ emphasis,
+ isInsideComponent,
+ isProbablyScene,
+ fullyVisible,
+ isFocusedComponent,
+ isFocusableComponent,
+ isHighlightedForInteraction,
+ isDescendantOfSelected,
+ isErroredGroup,
+ colorTheme,
+ } = params
let styleType: StyleType = 'default'
let selectedType: SelectedType = 'unselected'
@@ -224,15 +241,16 @@ const computeResultingStyle = (
let result = getColors(styleType, selectedType, colorTheme)
- const borderRadiusTop = selected ? '5px 5px' : '0 0'
- // TODO: add the case of a last descendant of a selected component
- const borderRadiusBottom = selected && isSingleItem ? '5px 5px' : '0 0'
- const borderRadius = `${borderRadiusTop} ${borderRadiusBottom}`
+ const borderRadiusTop = isTopOfSelection ? NavigatorRowBorderRadius : 0
+ const borderRadiusBottom = isBottomOfSelection ? NavigatorRowBorderRadius : 0
result.style = {
...result.style,
fontWeight: isProbablyParentOfSelected || isProbablyScene ? 600 : 'inherit',
- borderRadius: borderRadius,
+ borderTopLeftRadius: borderRadiusTop,
+ borderTopRightRadius: borderRadiusTop,
+ borderBottomLeftRadius: borderRadiusBottom,
+ borderBottomRightRadius: borderRadiusBottom,
}
return result
@@ -568,8 +586,6 @@ export const NavigatorItem: React.FunctionComponent<
)
const isConditional = codeItemType === 'conditional'
- const isRemixItem = codeItemType === 'remix'
- const isCodeItem = codeItemType !== 'none'
const metadata = useEditorState(
Substores.metadata,
@@ -648,11 +664,6 @@ export const NavigatorItem: React.FunctionComponent<
[dispatch, navigatorEntry, selected, isHighlighted],
)
- const removeHighlight = React.useCallback(
- () => dispatch([EditorActions.clearHighlightedViews()]),
- [dispatch],
- )
-
const focusComponent = React.useCallback(
(event: React.MouseEvent) => {
if (isManuallyFocusableComponent && !event.altKey) {
@@ -680,7 +691,6 @@ export const NavigatorItem: React.FunctionComponent<
const isPlaceholder = isEntryAPlaceholder(props.navigatorEntry)
- const isComponentScene = useIsProbablyScene(navigatorEntry) && childComponentCount === 1
const isRenderProp = isRenderPropNavigatorEntry(navigatorEntry)
const containerStyle: React.CSSProperties = React.useMemo(() => {
@@ -702,25 +712,28 @@ export const NavigatorItem: React.FunctionComponent<
)
}, [childComponentCount, isFocusedComponent, isConditional])
- const isSingleItem = (canBeExpanded && collapsed) || childComponentCount === 0
-
- const resultingStyle = computeResultingStyle(
- elementIsData ? false : selected,
- emphasis,
- isInsideComponent,
- isDynamic,
- isProbablyScene,
- fullyVisible,
- isFocusedComponent,
- isInFocusedComponentSubtree,
- isManuallyFocusableComponent,
- isHighlightedForInteraction,
- elementIsData && selected ? false : isDescendantOfSelected,
- isErroredGroup,
- colorTheme,
- isSingleItem,
+ const { isTopOfSelection, isBottomOfSelection } = useNavigatorSelectionBoundsForEntry(
+ navigatorEntry,
+ selected,
+ childComponentCount,
)
+ const resultingStyle = computeResultingStyle({
+ selected: elementIsData ? false : selected,
+ isTopOfSelection: isTopOfSelection,
+ isBottomOfSelection: isBottomOfSelection,
+ emphasis: emphasis,
+ isInsideComponent: isInsideComponent,
+ isProbablyScene: isProbablyScene,
+ fullyVisible: fullyVisible,
+ isFocusedComponent: isFocusedComponent,
+ isFocusableComponent: isManuallyFocusableComponent,
+ isHighlightedForInteraction: isHighlightedForInteraction,
+ isDescendantOfSelected: elementIsData && selected ? false : isDescendantOfSelected,
+ isErroredGroup: isErroredGroup,
+ colorTheme: colorTheme,
+ })
+
const rowStyle = useKeepReferenceEqualityIfPossible({
paddingLeft: getElementPadding(entryNavigatorDepth),
height: getItemHeight(navigatorEntry),
diff --git a/editor/src/components/navigator/navigator-item/use-navigator-selection-bounds-for-entry.tsx b/editor/src/components/navigator/navigator-item/use-navigator-selection-bounds-for-entry.tsx
new file mode 100644
index 000000000000..43ed0956a51b
--- /dev/null
+++ b/editor/src/components/navigator/navigator-item/use-navigator-selection-bounds-for-entry.tsx
@@ -0,0 +1,77 @@
+import React from 'react'
+import * as EP from '../../../core/shared/element-path'
+import type { NavigatorEntry } from '../../editor/store/editor-state'
+import { Substores, useEditorState } from '../../editor/store/store-hook'
+import { isRegulaNavigatorRow } from '../navigator-row'
+
+export function useNavigatorSelectionBoundsForEntry(
+ navigatorEntry: NavigatorEntry,
+ selected: boolean,
+ childComponentCount: number,
+): {
+ isTopOfSelection: boolean
+ isBottomOfSelection: boolean
+} {
+ const selectedViews = useEditorState(
+ Substores.selectedViews,
+ (store) => store.editor.selectedViews,
+ 'useNavigatorSelectionBoundsCheck selectedViews',
+ )
+
+ const navigatorRowsPaths = useEditorState(
+ Substores.derived,
+ (store) => {
+ return store.derived.navigatorRows.map((row) =>
+ isRegulaNavigatorRow(row) ? row.entry.elementPath : row.entries[0].elementPath,
+ )
+ },
+ 'useNavigatorSelectionBoundsCheck navigatorRowsPaths',
+ )
+
+ const collapsedViews = useEditorState(
+ Substores.navigator,
+ (store) => {
+ return store.editor.navigator.collapsedViews
+ },
+ 'useNavigatorSelectionBoundsCheck collapsedViews',
+ )
+
+ return React.useMemo(() => {
+ const index = navigatorRowsPaths.findIndex((view) =>
+ EP.pathsEqual(view, navigatorEntry.elementPath),
+ )
+
+ const previous = index > 0 ? navigatorRowsPaths.at(index - 1) : null
+ const next = index < navigatorRowsPaths.length - 1 ? navigatorRowsPaths.at(index + 1) : null
+
+ const isDangling =
+ childComponentCount === 0 || collapsedViews.includes(navigatorEntry.elementPath)
+
+ const isTopOfSelection =
+ previous == null ||
+ (!selectedViews.some(
+ (view) =>
+ EP.pathsEqual(view, previous) ||
+ EP.isDescendantOf(EP.parentPath(navigatorEntry.elementPath), view) ||
+ EP.isDescendantOf(previous, view),
+ ) &&
+ selected)
+
+ const isBottomOfSelection =
+ next == null ||
+ ((!selected || isDangling) &&
+ !selectedViews.some((view) => EP.pathsEqual(view, next) || EP.isDescendantOf(next, view)))
+
+ return {
+ isTopOfSelection: isTopOfSelection,
+ isBottomOfSelection: isBottomOfSelection,
+ }
+ }, [
+ navigatorRowsPaths,
+ navigatorEntry,
+ selectedViews,
+ selected,
+ childComponentCount,
+ collapsedViews,
+ ])
+}