Skip to content

Commit

Permalink
Consistent border radii for navigator selection (#5928)
Browse files Browse the repository at this point in the history
**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.

<img width="926" alt="Screenshot 2024-06-13 at 4 08 41 PM"
src="https://github.com/concrete-utopia/utopia/assets/1081051/eee83821-3302-46e5-a98d-9d1f4bd9e6bc">

**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
  • Loading branch information
ruggi authored Jun 13, 2024
1 parent b56b5e6 commit 58a7092
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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(
Expand Down Expand Up @@ -87,6 +92,12 @@ export const CondensedEntryItemWrapper = React.memo(
)
}, [selectedViews, props.navigatorRow])

const { isTopOfSelection, isBottomOfSelection } = useNavigatorSelectionBoundsForEntry(
props.navigatorRow.entries[0],
rowRootSelected,
0,
)

return (
<div
style={{
Expand All @@ -98,12 +109,16 @@ export const CondensedEntryItemWrapper = React.memo(
: hasSelection || wholeRowInsideSelection
? colorTheme.childSelectionBlue.value
: 'transparent',
borderTopLeftRadius: rowContainsSelection ? 5 : 0,
borderTopRightRadius: rowContainsSelection ? 5 : 0,
borderTopLeftRadius: isTopOfSelection ? NavigatorRowBorderRadius : 0,
borderTopRightRadius: isTopOfSelection ? NavigatorRowBorderRadius : 0,
borderBottomLeftRadius:
rowContainsSelection && (isCollapsed || isDataReferenceRow) ? 5 : 0,
isBottomOfSelection && (isCollapsed || isDataReferenceRow)
? NavigatorRowBorderRadius
: 0,
borderBottomRightRadius:
rowContainsSelection && (isCollapsed || isDataReferenceRow) ? 5 : 0,
isBottomOfSelection && (isCollapsed || isDataReferenceRow)
? NavigatorRowBorderRadius
: 0,
overflowX: 'auto',
}}
>
Expand Down
105 changes: 59 additions & 46 deletions editor/src/components/navigator/navigator-item/navigator-item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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'

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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(() => {
Expand All @@ -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),
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
])
}

0 comments on commit 58a7092

Please sign in to comment.