From 770b1cef7d4f98ef203db717c89742b352817f68 Mon Sep 17 00:00:00 2001 From: Federico Ruggi <1081051+ruggi@users.noreply.github.com> Date: Mon, 10 Jun 2024 13:33:46 +0200 Subject: [PATCH] Revamped navigator list cartouche (#5866) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **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 Screenshot 2024-06-07 at 6 17 35 PM **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 --- .../component-section/cartouche-ui.tsx | 3 + .../data-reference-cartouche.tsx | 2 + .../layout-section/list-source-cartouche.tsx | 5 ++ .../navigator/navigator-item/map-counter.tsx | 62 +++++++++---------- .../navigator-item.spec.browser2.tsx | 3 +- .../navigator-item/navigator-item.tsx | 14 ++--- 6 files changed, 46 insertions(+), 43 deletions(-) diff --git a/editor/src/components/inspector/sections/component-section/cartouche-ui.tsx b/editor/src/components/inspector/sections/component-section/cartouche-ui.tsx index 343635ab711d..adcbe96f16a0 100644 --- a/editor/src/components/inspector/sections/component-section/cartouche-ui.tsx +++ b/editor/src/components/inspector/sections/component-section/cartouche-ui.tsx @@ -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( @@ -127,6 +128,8 @@ export const CartoucheUI = React.forwardRef( // a trailing ellipsis is added to indicate that the object can be traversed …, )} + {/* badge (this check is redundant but it should make it more clear that this is optional) */} + {when(props.badge != null, props.badge)} diff --git a/editor/src/components/inspector/sections/component-section/data-reference-cartouche.tsx b/editor/src/components/inspector/sections/component-section/data-reference-cartouche.tsx index 4cf40eb05ddc..a4462d7d67aa 100644 --- a/editor/src/components/inspector/sections/component-section/data-reference-cartouche.tsx +++ b/editor/src/components/inspector/sections/component-section/data-reference-cartouche.tsx @@ -173,6 +173,7 @@ interface DataCartoucheInnerProps { hideTooltip?: boolean datatype: CartoucheDataType highlight?: CartoucheHighlight | null + badge?: React.ReactNode } export const DataCartoucheInner = React.forwardRef( @@ -220,6 +221,7 @@ export const DataCartoucheInner = React.forwardRef( role='selection' source={source} ref={ref} + badge={props.badge} > {contentsToDisplay.label ?? 'DATA'} diff --git a/editor/src/components/inspector/sections/layout-section/list-source-cartouche.tsx b/editor/src/components/inspector/sections/layout-section/list-source-cartouche.tsx index b97383c719da..6f39e7163962 100644 --- a/editor/src/components/inspector/sections/layout-section/list-source-cartouche.tsx +++ b/editor/src/components/inspector/sections/layout-section/list-source-cartouche.tsx @@ -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) { @@ -64,6 +65,7 @@ interface MapListSourceCartoucheProps { target: ElementPath selected: boolean openOn: 'single-click' | 'double-click' + countChildren?: boolean } export const MapListSourceCartoucheNavigator = React.memo((props: MapListSourceCartoucheProps) => { @@ -223,6 +225,9 @@ const MapListSourceCartoucheInner = React.memo((props: MapListSourceCartouchePro testId='list-source-cartouche' contentIsComingFromServer={isDataComingFromHookResult} datatype={cartoucheDataType} + badge={ + props.countChildren ? : null + } /> ) diff --git a/editor/src/components/navigator/navigator-item/map-counter.tsx b/editor/src/components/navigator/navigator-item/map-counter.tsx index f9c6c502017c..c7acaf17f7a9 100644 --- a/editor/src/components/navigator/navigator-item/map-counter.tsx +++ b/editor/src/components/navigator/navigator-item/map-counter.tsx @@ -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 { @@ -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-' @@ -27,27 +26,19 @@ 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 = @@ -55,18 +46,16 @@ export const MapCounter = React.memo((props: MapCounterProps) => { ? 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, } @@ -74,6 +63,14 @@ export const MapCounter = React.memo((props: MapCounterProps) => { '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 @@ -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 (
{shownCounterValue} @@ -115,6 +110,7 @@ export const MapCounter = React.memo((props: MapCounterProps) => { }) function getMapCounterStyleProps( + colorTheme: ThemeObject, overrideStatus: OverrideStatus, selectedStatus: SelectedStatus, ): CSSProperties { @@ -125,7 +121,7 @@ function getMapCounterStyleProps( height: 15, width: 'max-content', minWidth: 15, - borderRadius: 15, + borderRadius: 4, padding: 4, fontWeight: 400, marginRight: 2, @@ -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 { diff --git a/editor/src/components/navigator/navigator-item/navigator-item.spec.browser2.tsx b/editor/src/components/navigator/navigator-item/navigator-item.spec.browser2.tsx index b7b69de31ab7..ae03a9a349c8 100644 --- a/editor/src/components/navigator/navigator-item/navigator-item.spec.browser2.tsx +++ b/editor/src/components/navigator/navigator-item/navigator-item.spec.browser2.tsx @@ -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() @@ -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') diff --git a/editor/src/components/navigator/navigator-item/navigator-item.tsx b/editor/src/components/navigator/navigator-item/navigator-item.tsx index 73df1ee99cc7..ee39a0c6208f 100644 --- a/editor/src/components/navigator/navigator-item/navigator-item.tsx +++ b/editor/src/components/navigator/navigator-item/navigator-item.tsx @@ -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, @@ -1242,7 +1241,12 @@ export const NavigatorRowLabel = React.memo((props: NavigatorRowLabelProps) => { />, )} -
+
{ remixItemType={props.remixItemType} />
-