Skip to content

Commit

Permalink
Short Cartouches (#5925)
Browse files Browse the repository at this point in the history
**Problem:**
The cartouches in the navigator and the inspector don't currently have a
max width and they can be overlong.

**Fix:**
For property and element accessors, only show the last bit.

<img width="340" alt="image"
src="https://github.com/concrete-utopia/utopia/assets/2226774/7229048e-bcc8-445e-8f73-53a345bf97a5">
<img width="243" alt="image"
src="https://github.com/concrete-utopia/utopia/assets/2226774/e269aae4-abc4-4185-8454-44372497ec03">
<img width="275" alt="image"
src="https://github.com/concrete-utopia/utopia/assets/2226774/08e2c030-3891-44d7-9d91-b53ec2a06f9c">


**Commit Details:**

- Added `shortLabel` to `getTextContentOfElement`
- If the DataReferenceCartouche has a non-null shortLabel, it is used
- The tooltip always shows the long label

**Bonus**
The PR also changes when do we hide the cartouche icon – the new ruleset
only hides the cartouche icon for inline value literals such as
`myProp="value"` and `<div>{'myValue'}</div>`
  • Loading branch information
balazsbajorics authored Jun 13, 2024
1 parent 7cb0904 commit de7afad
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/** @jsx jsx */
import React from 'react'
import { jsx } from '@emotion/react'
import type { DataReferenceCartoucheContentType } from './data-reference-cartouche'
import { DataCartoucheInner } from './data-reference-cartouche'
import { NO_OP } from '../../../../core/shared/utils'
import type { ElementPath, PropertyPath } from '../../../../core/shared/project-file-types'
Expand All @@ -12,7 +13,11 @@ import type { CartoucheDataType } from './cartouche-ui'
import { useColorTheme } from '../../../../uuiui'

interface IdentifierExpressionCartoucheControlProps {
contents: string
contents: {
type: DataReferenceCartoucheContentType
label: string | null
shortLabel: string | null
}
icon: React.ReactChild
matchType: 'full' | 'partial'
onOpenDataPicker: () => void
Expand Down Expand Up @@ -42,7 +47,7 @@ export const IdentifierExpressionCartoucheControl = React.memo(
return (
<CartoucheInspectorWrapper>
<DataCartoucheInner
contentsToDisplay={{ label: props.contents, type: 'reference' }}
contentsToDisplay={props.contents}
onClick={NO_OP}
selected={false}
onDoubleClick={onOpenDataPicker}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ export interface HoverHandlers {

export type CartoucheDataType = 'renderable' | 'boolean' | 'array' | 'object' | 'unknown'

export type CartoucheSource = 'internal' | 'external' | 'literal'
export type CartoucheSource = 'internal' | 'external' | 'inline-literal' | 'literal-assignment'
export type CartoucheHighlight = 'strong' | 'subtle' | 'disabled'

export type CartoucheUIProps = React.PropsWithChildren<{
tooltip?: string | null
tooltip: string | null
source: CartoucheSource
role: 'selection' | 'information' | 'folder'
datatype: CartoucheDataType
Expand Down Expand Up @@ -121,7 +121,7 @@ export const CartoucheUI = React.forwardRef(
},
}}
>
{source === 'literal' ? null : (
{source === 'inline-literal' ? null : (
<Icn
category='navigator-element'
type={dataTypeToIconType(datatype)}
Expand All @@ -141,12 +141,6 @@ export const CartoucheUI = React.forwardRef(
overflow: 'hidden',
textOverflow: 'ellipsis',

/* Beginning of string */
...(shouldUseRtlCSS
? {
direction: source === 'literal' ? 'ltr' : 'rtl', // TODO we need a better way to ellipsize the beginning because rtl eats ' " marks
}
: {}),
textAlign: 'left',
...(role !== 'information' ? UtopiaStyles.fontStyles.monospaced : {}),
display: 'flex',
Expand Down Expand Up @@ -250,7 +244,8 @@ function useCartoucheColors(source: CartoucheSource, highlight: CartoucheHighlig
},
icon: { default: 'dynamic', hovered: 'dynamic', selected: 'on-highlight-main' },
}
case 'literal':
case 'inline-literal':
case 'literal-assignment':
return {
fg: {
default: colorTheme.fg1.value,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { isJSXElement, isJSXTextBlock } from '../../../../core/shared/element-te
import { IdentifierExpressionCartoucheControl } from './cartouche-control'
import { renderedAtChildNode } from '../../../editor/store/editor-state'
import { isRight } from '../../../../core/shared/either'
import { DataReferenceCartoucheControl } from './data-reference-cartouche'
import { DataReferenceCartoucheControl, getTextContentOfElement } from './data-reference-cartouche'
import { assertNever } from '../../../../core/shared/utils'
import { childrenAreProbablyNumericExpression } from '../../../editor/element-children'
import type { CartoucheDataType } from './cartouche-ui'
Expand Down Expand Up @@ -61,7 +61,7 @@ export function useChildrenPropOverride(
case 'expression':
return (
<IdentifierExpressionCartoucheControl
contents={'Expression'}
contents={{ type: 'object-literal', label: 'Expression', shortLabel: null }}
icon={React.createElement(iconForControlType('none'))}
matchType='partial'
onOpenDataPicker={props.onOpenDataPicker}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ import {
childrenAreProbablyNumericExpression,
replaceFirstChildAndDeleteSiblings,
} from '../../../editor/element-children'
import { getTextContentOfElement } from './data-reference-cartouche'

export interface PropertyLabelAndPlusButtonProps {
title: string
Expand Down Expand Up @@ -255,7 +256,7 @@ const ControlForProp = React.memo((props: ControlForPropProps<RegularControlDesc
) {
return (
<IdentifierExpressionCartoucheControl
contents={jsxElementChildToText(attributeExpression, null, null, 'jsx', 'inner')}
contents={getTextContentOfElement(attributeExpression, null)}
icon={React.createElement(iconForControlType(props.controlDescription.control))}
matchType='full'
onOpenDataPicker={props.onOpenDataPicker}
Expand All @@ -277,7 +278,7 @@ const ControlForProp = React.memo((props: ControlForPropProps<RegularControlDesc
if (controlDescription.control !== 'jsx') {
return (
<IdentifierExpressionCartoucheControl
contents={'Expression'}
contents={getTextContentOfElement(attributeExpression, null)}
icon={React.createElement(iconForControlType('none'))}
matchType='partial'
onOpenDataPicker={props.onOpenDataPicker}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ interface DataReferenceCartoucheControlProps {
selected: boolean
renderedAt: RenderedAt
surroundingScope: ElementPath
hideTooltip?: boolean
highlight?: CartoucheHighlight | null
}

Expand Down Expand Up @@ -158,7 +157,6 @@ export const DataReferenceCartoucheControl = React.memo(
onDelete={NO_OP}
testId={`data-reference-cartouche-${EP.toString(elementPath)}`}
contentIsComingFromServer={isDataComingFromHookResult}
hideTooltip={props.hideTooltip}
datatype={currentlySelectedValueDataType}
/>
{/* this div prevents the popup form putting padding into the condensed rows */}
Expand All @@ -170,16 +168,20 @@ export const DataReferenceCartoucheControl = React.memo(
},
)

export type DataReferenceCartoucheContentType = 'value-literal' | 'object-literal' | 'reference'
interface DataCartoucheInnerProps {
onClick: (e: React.MouseEvent) => void
onDoubleClick: (e: React.MouseEvent) => void
selected: boolean
contentsToDisplay: { type: 'literal' | 'reference'; label: string | null }
contentsToDisplay: {
type: DataReferenceCartoucheContentType
label: string | null
shortLabel: string | null
}
safeToDelete: boolean
onDelete: () => void
testId: string
contentIsComingFromServer: boolean
hideTooltip?: boolean
datatype: CartoucheDataType
highlight?: CartoucheHighlight | null
badge?: React.ReactNode
Expand Down Expand Up @@ -211,8 +213,10 @@ export const DataCartoucheInner = React.forwardRef(
const onDelete = safeToDelete ? onDeleteInner : undefined

const source: CartoucheUIProps['source'] =
contentsToDisplay.type === 'literal'
? 'literal'
contentsToDisplay.type === 'value-literal'
? 'inline-literal'
: contentsToDisplay.type === 'object-literal'
? 'internal'
: contentIsComingFromServer
? 'external'
: 'internal'
Expand All @@ -226,13 +230,13 @@ export const DataCartoucheInner = React.forwardRef(
selected={selected}
highlight={highlight}
testId={testId}
tooltip={!props.hideTooltip ? contentsToDisplay.label ?? 'DATA' : null}
tooltip={contentsToDisplay.label ?? contentsToDisplay.shortLabel ?? 'DATA'}
role='selection'
source={source}
ref={ref}
badge={props.badge}
>
{contentsToDisplay.label ?? 'DATA'}
{contentsToDisplay.shortLabel ?? contentsToDisplay.label ?? 'DATA'}
</CartoucheUI>
)
},
Expand All @@ -241,46 +245,55 @@ export const DataCartoucheInner = React.forwardRef(
export function getTextContentOfElement(
element: JSXElementChild,
metadata: ElementInstanceMetadata | null,
): { type: 'literal' | 'reference'; label: string | null } {
): {
type: DataReferenceCartoucheContentType
label: string | null
shortLabel: string | null
} {
switch (element.type) {
case 'ATTRIBUTE_VALUE':
return { type: 'literal', label: `${JSON.stringify(element.value)}` }
return { type: 'value-literal', label: `${JSON.stringify(element.value)}`, shortLabel: null }
case 'JSX_TEXT_BLOCK':
return { type: 'literal', label: element.text.trim() }
return { type: 'value-literal', label: element.text.trim(), shortLabel: null }
case 'JS_IDENTIFIER':
return { type: 'reference', label: element.name.trim() }
return { type: 'reference', label: element.name.trim(), shortLabel: null }
case 'JS_ELEMENT_ACCESS':
return {
type: 'reference',
label: `${getTextContentOfElement(element.onValue, null).label}[${
getTextContentOfElement(element.element, null).label
}]`,
shortLabel: `${TruncationPrefix}${getTextContentOfElement(element.element, null).label}`,
}
case 'JS_PROPERTY_ACCESS':
return {
type: 'reference',
label: `${getTextContentOfElement(element.onValue, null).label}.${element.property}`,
shortLabel: `${TruncationPrefix}${element.property}`,
}
case 'ATTRIBUTE_FUNCTION_CALL':
return { type: 'reference', label: `${element.functionName}(...` }
return { type: 'reference', label: `${element.functionName}(...`, shortLabel: null }
case 'JSX_ELEMENT':
return {
type: 'literal',
type: 'object-literal',
label: metadata?.textContent ?? `${getJSXElementNameLastPart(element.name)}`,
shortLabel: null,
}
case 'ATTRIBUTE_NESTED_ARRAY':
return { type: 'literal', label: '[...]' }
return { type: 'object-literal', label: '[...]', shortLabel: null }
case 'ATTRIBUTE_NESTED_OBJECT':
return { type: 'literal', label: '{...}' }
return { type: 'object-literal', label: '{...}', shortLabel: null }
case 'JSX_MAP_EXPRESSION':
return { type: 'literal', label: 'List' }
return { type: 'object-literal', label: 'List', shortLabel: null }
case 'JSX_CONDITIONAL_EXPRESSION':
return { type: 'literal', label: 'Conditional' }
return { type: 'object-literal', label: 'Conditional', shortLabel: null }
case 'ATTRIBUTE_OTHER_JAVASCRIPT':
return { type: 'literal', label: element.originalJavascript }
return { type: 'object-literal', label: element.originalJavascript, shortLabel: null }
case 'JSX_FRAGMENT':
return { type: 'literal', label: 'Fragment' }
return { type: 'object-literal', label: 'Fragment', shortLabel: null }
default:
assertNever(element)
}
}

const TruncationPrefix = `…`
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export const DataPickerCartouche = React.memo(
return (
<CartoucheUI
key={data.valuePath.toString()}
tooltip={null}
source={dataSource ?? 'internal'}
datatype={childTypeToCartoucheDataType(data.type)}
selected={variableMatches(data.variableInfo) && selected}
Expand Down Expand Up @@ -56,7 +57,9 @@ export const DataPickerCartouche = React.memo(
},
)

export function useVariableDataSource(variable: DataPickerOption | null) {
export function useVariableDataSource(
variable: DataPickerOption | null,
): CartoucheUIProps['source'] | null {
return useEditorState(
Substores.projectContentsAndMetadata,
(store) => {
Expand All @@ -77,7 +80,7 @@ export function useVariableDataSource(variable: DataPickerOption | null) {
return 'external'
case 'literal-attribute':
case 'literal-assignment':
return 'literal'
return 'literal-assignment'
case 'component-prop':
case 'element-at-scope':
case 'failed':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,6 @@ const CondensedEntryItemContent = React.memo(
highlight={
props.rowRootSelected ? 'strong' : props.wholeRowInsideSelection ? 'subtle' : null
}
hideTooltip={true}
/>,
)}
</div>
Expand Down

0 comments on commit de7afad

Please sign in to comment.