Skip to content

Commit

Permalink
Revamped navigator list cartouche (#5866)
Browse files Browse the repository at this point in the history
**Problem:**

The navigator cartouche for list items needs some tidying up.

**Fix:**

- Remove its label
- Move the counter inside the cartouche itself
- Tweak the colors a bit

<img width="758" alt="Screenshot 2024-06-07 at 6 17 35 PM"
src="https://github.com/concrete-utopia/utopia/assets/1081051/2fa6e7d9-f046-41f2-bc21-d8512a26104f">

**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 #5843
  • Loading branch information
ruggi authored Jun 10, 2024
1 parent e5c5e49 commit 770b1ce
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export type CartoucheUIProps = React.PropsWithChildren<{
onClick?: (e: React.MouseEvent) => void
onDoubleClick?: (e: React.MouseEvent) => void
onHover?: HoverHandlers
badge?: React.ReactNode
}>

export const CartoucheUI = React.forwardRef(
Expand Down Expand Up @@ -127,6 +128,8 @@ export const CartoucheUI = React.forwardRef(
// a trailing ellipsis is added to indicate that the object can be traversed
<span></span>,
)}
{/* badge (this check is redundant but it should make it more clear that this is optional) */}
{when(props.badge != null, props.badge)}
</FlexRow>
</Tooltip>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ interface DataCartoucheInnerProps {
hideTooltip?: boolean
datatype: CartoucheDataType
highlight?: CartoucheHighlight | null
badge?: React.ReactNode
}

export const DataCartoucheInner = React.forwardRef(
Expand Down Expand Up @@ -220,6 +221,7 @@ export const DataCartoucheInner = React.forwardRef(
role='selection'
source={source}
ref={ref}
badge={props.badge}
>
{contentsToDisplay.label ?? 'DATA'}
</CartoucheUI>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { mapDropNulls } from '../../../../core/shared/array-utils'
import { traceDataFromElement, dataPathSuccess } from '../../../../core/data-tracing/data-tracing'
import type { CartoucheDataType } from '../component-section/cartouche-ui'
import { CartoucheInspectorWrapper } from '../component-section/cartouche-control'
import { MapCounter } from '../../../navigator/navigator-item/map-counter'

function filterVariableOption(option: DataPickerOption): DataPickerOption | null {
switch (option.type) {
Expand Down Expand Up @@ -64,6 +65,7 @@ interface MapListSourceCartoucheProps {
target: ElementPath
selected: boolean
openOn: 'single-click' | 'double-click'
countChildren?: boolean
}

export const MapListSourceCartoucheNavigator = React.memo((props: MapListSourceCartoucheProps) => {
Expand Down Expand Up @@ -223,6 +225,9 @@ const MapListSourceCartoucheInner = React.memo((props: MapListSourceCartouchePro
testId='list-source-cartouche'
contentIsComingFromServer={isDataComingFromHookResult}
datatype={cartoucheDataType}
badge={
props.countChildren ? <MapCounter selected={props.selected} elementPath={target} /> : null
}
/>
</React.Fragment>
)
Expand Down
62 changes: 28 additions & 34 deletions editor/src/components/navigator/navigator-item/map-counter.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import React from 'react'
import type { CSSProperties } from 'react'
import type { NavigatorEntry } from '../../../components/editor/store/editor-state'
import { isRegularNavigatorEntry } from '../../../components/editor/store/editor-state'
import { Substores, useEditorState } from '../../editor/store/store-hook'
import { MetadataUtils } from '../../../core/model/element-metadata-utils'
import { colorTheme } from '../../../uuiui'
import * as EP from '../../../core/shared/element-path'
import type { ElementPath } from '../../../core/shared/project-file-types'
import {
Expand All @@ -14,8 +11,10 @@ import {
import { isRight } from '../../../core/shared/either'
import { isJSXMapExpression } from '../../../core/shared/element-template'
import { assertNever } from '../../../core/shared/utils'
import type { EditorDispatch } from '../../editor/action-types'
import { setMapCountOverride } from '../../editor/actions/action-creators'
import { useDispatch } from '../../editor/store/dispatch-context'
import { useColorTheme } from '../../../uuiui'
import type { ThemeObject } from '../../../uuiui/styles/theme/theme-helpers'

export const MapCounterTestIdPrefix = 'map-counter-'

Expand All @@ -27,53 +26,51 @@ type OverrideStatus = 'no-override' | 'overridden' | 'override-failed'
type SelectedStatus = true | false

interface MapCounterProps {
navigatorEntry: NavigatorEntry
dispatch: EditorDispatch
elementPath: ElementPath
selected: boolean
}

export const MapCounter = React.memo((props: MapCounterProps) => {
const { navigatorEntry, dispatch } = props
const { elementPath } = navigatorEntry
const dispatch = useDispatch()

const { nrChildren, countOverride } = useEditorState(
const counter = useEditorState(
Substores.metadata,
(store) => {
if (!isRegularNavigatorEntry(navigatorEntry)) {
return {
nrChildren: null,
countOverride: null,
}
}
const elementMetadata = MetadataUtils.findElementByElementPath(
store.editor.jsxMetadata,
elementPath,
props.elementPath,
)

const element =
elementMetadata != null && isRight(elementMetadata.element)
? elementMetadata.element.value
: null
if (element == null || !isJSXMapExpression(element)) {
return {
nrChildren: null,
countOverride: null,
}
return null
}

const commentFlag = findUtopiaCommentFlag(element.comments, 'map-count')
const mapCountOverride = isUtopiaCommentFlagMapCount(commentFlag) ? commentFlag.value : null
return {
nrChildren: MetadataUtils.getChildrenOrdered(
store.editor.jsxMetadata,
store.editor.elementPathTree,
elementPath,
props.elementPath,
).length,
countOverride: mapCountOverride,
}
},
'MapCounter counterValue',
)

const nrChildren = React.useMemo(() => {
return counter?.nrChildren ?? null
}, [counter])

const countOverride = React.useMemo(() => {
return counter?.countOverride ?? null
}, [counter])

const isOverridden = nrChildren != null && countOverride != null
const shownCounterValue = countOverride ?? nrChildren
const overrideFailed = isOverridden && countOverride > nrChildren
Expand All @@ -93,20 +90,18 @@ export const MapCounter = React.memo((props: MapCounterProps) => {
}
const nextValue = getNextOverrideValue(overrideStatus, countOverride, nrChildren)
if (nextValue !== countOverride) {
dispatch([setMapCountOverride(elementPath, nextValue)])
dispatch([setMapCountOverride(props.elementPath, nextValue)])
}
}, [elementPath, dispatch, overrideStatus, countOverride, nrChildren])

if (nrChildren == null) {
return null
}
}, [props.elementPath, dispatch, overrideStatus, countOverride, nrChildren])

const selectedStatus = props.selected

const colorTheme = useColorTheme()

return (
<div
data-testid={getMapCounterTestId(elementPath)}
style={getMapCounterStyleProps(overrideStatus, selectedStatus)}
data-testid={getMapCounterTestId(props.elementPath)}
style={getMapCounterStyleProps(colorTheme, overrideStatus, selectedStatus)}
onClick={onClick}
>
{shownCounterValue}
Expand All @@ -115,6 +110,7 @@ export const MapCounter = React.memo((props: MapCounterProps) => {
})

function getMapCounterStyleProps(
colorTheme: ThemeObject,
overrideStatus: OverrideStatus,
selectedStatus: SelectedStatus,
): CSSProperties {
Expand All @@ -125,7 +121,7 @@ function getMapCounterStyleProps(
height: 15,
width: 'max-content',
minWidth: 15,
borderRadius: 15,
borderRadius: 4,
padding: 4,
fontWeight: 400,
marginRight: 2,
Expand All @@ -135,10 +131,8 @@ function getMapCounterStyleProps(
case 'no-override':
return {
...stylePropsBase,
backgroundColor: selectedStatus
? colorTheme.whiteOpacity30.value
: colorTheme.fg0Opacity10.value,
color: selectedStatus ? colorTheme.white.value : 'unset',
backgroundColor: selectedStatus ? colorTheme.whiteOpacity30.value : colorTheme.bg1.value,
color: selectedStatus ? colorTheme.white.value : colorTheme.brandNeonPink.value,
}
case 'overridden':
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,6 @@ describe('Navigator item row icons', () => {
const testId = itemLabelTestIdForEntry(navigatorEntry)
if (expectedLabel != null) {
const labelElement = editor.renderedDOM.getByTestId(testId)

expect(labelElement.innerText).toEqual(expectedLabel)
} else {
expect(() => editor.renderedDOM.getByTestId(testId)).toThrow()
Expand All @@ -345,7 +344,7 @@ describe('Navigator item row icons', () => {
await checkNavigatorLabel(visibleNavigatorTargets[11], null)
await checkNavigatorLabel(visibleNavigatorTargets[12], 'CODE')
await checkNavigatorLabel(visibleNavigatorTargets[13], 'div')
await checkNavigatorLabel(visibleNavigatorTargets[14], 'LIST')
await checkNavigatorLabel(visibleNavigatorTargets[14], 'List')
await checkNavigatorLabel(visibleNavigatorTargets[15], 'div')
await checkNavigatorLabel(visibleNavigatorTargets[16], 'Fragment')
await checkNavigatorLabel(visibleNavigatorTargets[17], 'div')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ import { LayoutIcon } from './layout-icon'
import { NavigatorItemActionSheet } from './navigator-item-components'
import { assertNever } from '../../../core/shared/utils'
import type { ElementPathTrees } from '../../../core/shared/element-path-tree'
import { MapCounter } from './map-counter'
import {
conditionalTarget,
renderPropTarget,
Expand Down Expand Up @@ -1242,7 +1241,12 @@ export const NavigatorRowLabel = React.memo((props: NavigatorRowLabelProps) => {
/>,
)}

<div style={{ textTransform: isCodeItem ? 'uppercase' : undefined }}>
<div
style={{
textTransform: isCodeItem ? 'uppercase' : undefined,
display: props.codeItemType === 'map' ? 'none' : 'block',
}}
>
<ItemLabel
key={`label-${props.label}`}
testId={`navigator-item-label-${props.label}`}
Expand All @@ -1255,15 +1259,11 @@ export const NavigatorRowLabel = React.memo((props: NavigatorRowLabelProps) => {
remixItemType={props.remixItemType}
/>
</div>
<MapCounter
navigatorEntry={props.navigatorEntry}
dispatch={props.dispatch}
selected={props.selected}
/>
<MapListSourceCartoucheNavigator
target={props.navigatorEntry.elementPath}
selected={props.selected}
openOn='double-click'
countChildren={true}
/>
<ComponentPreview
key={`preview-${props.label}`}
Expand Down

0 comments on commit 770b1ce

Please sign in to comment.