Skip to content

Commit

Permalink
Inspector UI Fixes (#6119)
Browse files Browse the repository at this point in the history
Making a number of updates to the inspector UI:
- reordering the "container" section to be below the frame section
- when the border-radius and padding controls are split into 4 inputs,
the last 2 move onto another row
- the split input control always stays in the right place
- when a dropdown is not hovered, the `v` expansion arrow stays at the
end of the text, and moves to the far right of the input only when it is
hovered
- the opacity control is not a scrubbable input, no longer a slider
- blend mode is always visible
- the overflow toggle is removed (since we already have clip content)
- the alignment buttons are above the style section
- other various padding and border styling tweaks

BK:
- updated the tests to use the opacity input instead of the opacity
slider
- updated the tests to look for the `Background` label instead of
`Container` (which was removed)

| Before  | After |
| ------------- | ------------- |
| <img width="289" alt="Screenshot 2024-07-25 at 5 38 28 PM"
src="https://github.com/user-attachments/assets/9fe8fa32-2f56-415e-91e2-f606a98bb7ba">
| <img width="303" alt="Screenshot 2024-07-25 at 5 38 49 PM"
src="https://github.com/user-attachments/assets/8fdedddf-08e1-4806-b4d9-947089ecdd45">
|
| <img width="274" alt="Screenshot 2024-07-25 at 5 41 07 PM"
src="https://github.com/user-attachments/assets/c6d7a264-c9a5-4eb8-9e76-9c69684ae51d">
| <img width="274" alt="Screenshot 2024-07-25 at 5 40 12 PM"
src="https://github.com/user-attachments/assets/6fa10dd2-bbd3-4243-9faf-0fe4488a9836">
|
| <img width="278" alt="Screenshot 2024-07-25 at 5 39 44 PM"
src="https://github.com/user-attachments/assets/262803d4-c308-4689-8e4a-19e340c8d231">
| <img width="269" alt="Screenshot 2024-07-25 at 5 40 39 PM"
src="https://github.com/user-attachments/assets/2c6b5796-337e-4cbc-a922-9974b34f66fa">
|
| <img width="274" alt="Screenshot 2024-07-25 at 5 42 43 PM"
src="https://github.com/user-attachments/assets/c79130dd-722c-4630-835c-d2a5e354b203">
| <img width="269" alt="Screenshot 2024-07-25 at 5 42 09 PM"
src="https://github.com/user-attachments/assets/ff6446f2-fd9a-4208-815e-853521239cfb">
|
|
![25-43-gxom7-gg29c](https://github.com/user-attachments/assets/2be5e421-096a-4e93-baa8-119898c299d5)
|
![25-43-crhaz-khuwd](https://github.com/user-attachments/assets/03481ca0-400d-4dd8-87bc-0da372186f4f)
|

**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

---------

Co-authored-by: Berci Kormendy <[email protected]>
  • Loading branch information
lankaukk and bkrmendy authored Jul 29, 2024
1 parent 4c44415 commit ca05c44
Show file tree
Hide file tree
Showing 20 changed files with 317 additions and 509 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ describe('inspector tests with real metadata', () => {
'padding-R',
)) as HTMLInputElement
const opacityControl = (await renderResult.renderedDOM.findByTestId(
'opacity-number-input',
'opacity',
)) as HTMLInputElement

matchInlineSnapshotBrowser(metadata.computedStyle?.['width'], `"203px"`)
Expand Down Expand Up @@ -707,7 +707,7 @@ describe('inspector tests with real metadata', () => {
`"simple"`,
)

matchInlineSnapshotBrowser(opacityControl.value, `"0.5"`)
matchInlineSnapshotBrowser(opacityControl.value, `"50%"`)
matchInlineSnapshotBrowser(
opacityControl.attributes.getNamedItemNS(null, 'data-controlstatus')?.value,
`"simple"`,
Expand Down Expand Up @@ -770,7 +770,7 @@ describe('inspector tests with real metadata', () => {
'radius-one',
)) as HTMLInputElement
const opacityControl = (await renderResult.renderedDOM.findByTestId(
'opacity-number-input',
'opacity',
)) as HTMLInputElement

matchInlineSnapshotBrowser(widthControl.value, `"0"`)
Expand Down Expand Up @@ -812,7 +812,7 @@ describe('inspector tests with real metadata', () => {
`"simple"`,
)

matchInlineSnapshotBrowser(opacityControl.value, `"1"`)
matchInlineSnapshotBrowser(opacityControl.value, `"100%"`)
matchInlineSnapshotBrowser(
opacityControl.attributes.getNamedItemNS(null, 'data-controlstatus')?.value,
`"simple"`,
Expand Down Expand Up @@ -1316,7 +1316,7 @@ describe('inspector tests with real metadata', () => {
'radius-one',
)) as HTMLInputElement
const opacityControl = (await renderResult.renderedDOM.findByTestId(
'opacity-number-input',
'opacity',
)) as HTMLInputElement

matchInlineSnapshotBrowser(widthControl.value, `"100"`)
Expand Down Expand Up @@ -1366,7 +1366,7 @@ describe('inspector tests with real metadata', () => {
)

matchInlineSnapshotBrowser(metadata.computedStyle?.['opacity'], `"0.5"`)
matchInlineSnapshotBrowser(opacityControl.value, `"0.5"`)
matchInlineSnapshotBrowser(opacityControl.value, `"50%"`)
matchInlineSnapshotBrowser(
opacityControl.attributes.getNamedItemNS(null, 'data-controlstatus')?.value,
`"controlled"`,
Expand Down Expand Up @@ -1419,7 +1419,7 @@ describe('inspector tests with real metadata', () => {
'padding-R',
)) as HTMLInputElement
const earlyOpacityControl = (await renderResult.renderedDOM.findByTestId(
'opacity-number-input',
'opacity',
)) as HTMLInputElement

matchInlineSnapshotBrowser(earlyMetadata.computedStyle?.['width'], `"203px"`)
Expand Down Expand Up @@ -1451,7 +1451,7 @@ describe('inspector tests with real metadata', () => {
`"detected-fromcss"`,
)

matchInlineSnapshotBrowser(earlyOpacityControl.value, `"0.5"`)
matchInlineSnapshotBrowser(earlyOpacityControl.value, `"50%"`)
matchInlineSnapshotBrowser(
earlyOpacityControl.attributes.getNamedItemNS(null, 'data-controlstatus')?.value,
`"detected-fromcss"`,
Expand Down Expand Up @@ -1490,7 +1490,7 @@ describe('inspector tests with real metadata', () => {
'padding-R',
)) as HTMLInputElement
const laterOpacityControl = (await renderResult.renderedDOM.findByTestId(
'opacity-number-input',
'opacity',
)) as HTMLInputElement

matchInlineSnapshotBrowser(laterMetadata.computedStyle?.['width'], `"203px"`)
Expand Down Expand Up @@ -1522,7 +1522,7 @@ describe('inspector tests with real metadata', () => {
`"simple"`,
)

matchInlineSnapshotBrowser(laterOpacityControl.value, `"0.5"`)
matchInlineSnapshotBrowser(laterOpacityControl.value, `"50%"`)
matchInlineSnapshotBrowser(
laterOpacityControl.attributes.getNamedItemNS(null, 'data-controlstatus')?.value,
`"simple"`,
Expand Down Expand Up @@ -1594,7 +1594,7 @@ describe('inspector tests with real metadata', () => {
'radius-one',
)) as HTMLInputElement
const opacityControl = (await renderResult.renderedDOM.findByTestId(
'opacity-number-input',
'opacity',
)) as HTMLInputElement

matchInlineSnapshotBrowser(widthControl.value, `"0"`)
Expand Down Expand Up @@ -1624,7 +1624,7 @@ describe('inspector tests with real metadata', () => {
`"simple"`,
)

matchInlineSnapshotBrowser(opacityControl.value, `"1"`)
matchInlineSnapshotBrowser(opacityControl.value, `"100%"`)
matchInlineSnapshotBrowser(
opacityControl.attributes.getNamedItemNS(null, 'data-controlstatus')?.value,
`"simple"`,
Expand Down Expand Up @@ -1873,7 +1873,7 @@ describe('inspector tests with real metadata', () => {
'radius-one',
)) as HTMLInputElement
const opacityControl = (await renderResult.renderedDOM.findByTestId(
'opacity-number-input',
'opacity',
)) as HTMLInputElement

matchInlineSnapshotBrowser(metadata.computedStyle?.['width'], `"250px"`)
Expand Down Expand Up @@ -1908,7 +1908,7 @@ describe('inspector tests with real metadata', () => {
)

matchInlineSnapshotBrowser(metadata.computedStyle?.['opacity'], `"0.3"`)
matchInlineSnapshotBrowser(opacityControl.value, `"0.3"`)
matchInlineSnapshotBrowser(opacityControl.value, `"30%"`)
matchInlineSnapshotBrowser(
opacityControl.attributes.getNamedItemNS(null, 'data-controlstatus')?.value,
`"detected-fromcss"`,
Expand Down Expand Up @@ -1998,7 +1998,7 @@ describe('inspector tests with real metadata', () => {
'radius-one',
)) as HTMLInputElement
const opacityControl = (await renderResult.renderedDOM.findByTestId(
'opacity-number-input',
'opacity',
)) as HTMLInputElement

// matchInlineSnapshotBrowser(metadata.computedStyle?.['minWidth'], `"0px"`)
Expand Down Expand Up @@ -2030,7 +2030,7 @@ describe('inspector tests with real metadata', () => {
)

matchInlineSnapshotBrowser(metadata.computedStyle?.['opacity'], `"1"`)
matchInlineSnapshotBrowser(opacityControl.value, `"1"`)
matchInlineSnapshotBrowser(opacityControl.value, `"100%"`)
matchInlineSnapshotBrowser(
opacityControl.attributes.getNamedItemNS(null, 'data-controlstatus')?.value,
`"detected-fromcss"`,
Expand Down Expand Up @@ -2108,7 +2108,7 @@ describe('inspector tests with real metadata', () => {
'radius-one',
)) as HTMLInputElement
const opacityControl = (await renderResult.renderedDOM.findByTestId(
'opacity-number-input',
'opacity',
)) as HTMLInputElement

// matchInlineSnapshotBrowser(metadata.computedStyle?.['minWidth'], `"0px"`)
Expand Down Expand Up @@ -2140,7 +2140,7 @@ describe('inspector tests with real metadata', () => {
)

matchInlineSnapshotBrowser(metadata.computedStyle?.['opacity'], `"1"`)
matchInlineSnapshotBrowser(opacityControl.value, `"1"`)
matchInlineSnapshotBrowser(opacityControl.value, `"100%"`)
matchInlineSnapshotBrowser(
opacityControl.attributes.getNamedItemNS(null, 'data-controlstatus')?.value,
`"detected"`,
Expand Down Expand Up @@ -4312,14 +4312,14 @@ describe('Undo behavior in inspector', () => {
})

const opacityControl = (await renderResult.renderedDOM.findByTestId(
'opacity-number-input',
'opacity',
)) as HTMLInputElement
matchInlineSnapshotBrowser(opacityControl.value, `"0.5"`)
matchInlineSnapshotBrowser(opacityControl.value, `"50%"`)

// change the opacity of the selected element
await setControlValue('opacity-number-input', '0.6', renderResult.renderedDOM)
await setControlValue('opacity', '60', renderResult.renderedDOM)

matchInlineSnapshotBrowser(opacityControl.value, `"0.6"`)
matchInlineSnapshotBrowser(opacityControl.value, `"60%"`)
matchInlineSnapshotBrowser(
opacityControl.attributes.getNamedItemNS(null, 'data-controlstatus')?.value,
`"simple"`,
Expand All @@ -4332,7 +4332,7 @@ describe('Undo behavior in inspector', () => {

// type a new value '0.8' to opacityControl
await act(async () => {
fireEvent.change(opacityControl, { target: { value: '0.8' } })
fireEvent.change(opacityControl, { target: { value: '80' } })
})

// press Cmd + Z on the opacityControl
Expand All @@ -4343,9 +4343,8 @@ describe('Undo behavior in inspector', () => {

// the control's value should now be undone
matchInlineSnapshotBrowser(
((await renderResult.renderedDOM.findByTestId('opacity-number-input')) as HTMLInputElement)
.value,
`"0.5"`,
((await renderResult.renderedDOM.findByTestId('opacity')) as HTMLInputElement).value,
`"50%"`,
)

// await for the code to be updated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const RootElementIndicator = () => {
justifyContent: 'center',
alignItems: 'center',
background: colorTheme.inspectorBackground.value,
height: 34,
}}
>
<div
Expand Down
5 changes: 4 additions & 1 deletion editor/src/components/inspector/inspector.spec.browser2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,10 @@ export var storyboard = (
})

const VisualSections = ['Transforms', 'Background', 'Border', 'Shadow', 'Text Shadow']
const LayoutSections = ['Frame', 'Container']
const LayoutSections = [
'Frame',
'α', // this is the label of the opacity input in what used to be the Container section
]
const LayoutSystemSections = ['Layout System']
const TypographySections = ['Type']

Expand Down
25 changes: 16 additions & 9 deletions editor/src/components/inspector/inspector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ import { DataReferenceSection } from './sections/data-reference-section'
import { replaceFirstChildAndDeleteSiblings } from '../editor/element-children'
import { InspectorSectionHeader } from './section-header'
import { GridPlacementSubsection } from './sections/style-section/container-subsection/grid-cell-subsection'
import { ContainerSubsection } from './sections/style-section/container-subsection/container-subsection'

export interface ElementPathElement {
name?: string
Expand Down Expand Up @@ -174,7 +175,8 @@ const AlignmentButtons = React.memo((props: { numberOfTargets: number }) => {
alignItems: 'center',
height: UtopiaTheme.layout.rowHeight.normal,
background: colorTheme.inspectorBackground.value,
padding: '8px 0',
padding: '8px 0px',
flexShrink: 0,
}}
>
<AlignDistributeButton
Expand Down Expand Up @@ -411,6 +413,9 @@ export const Inspector = React.memo<InspectorProps>((props: InspectorProps) => {
'Inspector shouldShowGridCellSection',
)

const shouldShowContainerSection =
selectedViews.length > 0 && inspectorPreferences.includes('layout')

function renderInspectorContents() {
return (
<React.Fragment>
Expand Down Expand Up @@ -450,9 +455,18 @@ export const Inspector = React.memo<InspectorProps>((props: InspectorProps) => {
paddingBottom: 50,
}}
>
{rootElementIsSelected ? (
<RootElementIndicator />
) : (
when(
shouldShowAlignmentButtons,
<AlignmentButtons numberOfTargets={selectedViews.length} />,
)
)}
{anyComponents || multiselectedContract === 'fragment' ? (
<ComponentSection isScene={false} />
) : null}

{unless(
shouldHideInspectorSections,
<InspectorSectionHeader
Expand All @@ -465,20 +479,13 @@ export const Inspector = React.memo<InspectorProps>((props: InspectorProps) => {
{when(
shouldShowStyleSectionContents,
<>
{rootElementIsSelected ? (
<RootElementIndicator />
) : (
when(
shouldShowAlignmentButtons,
<AlignmentButtons numberOfTargets={selectedViews.length} />,
)
)}
{when(multiselectedContract === 'fragment', <FragmentSection />)}
{when(
multiselectedContract !== 'fragment' && shouldShowSimplifiedLayoutSection,
// Position and Sizing sections are shown if Frame or Group is selected
<>
<SimplifiedLayoutSubsection />
{when(shouldShowContainerSection, <ContainerSubsection />)}
<ConstraintsSection />
</>,
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1534,7 +1534,6 @@ export const ComponentSectionInner = React.memo((props: ComponentSectionProps) =
padded={false}
variant='<----------1fr---------><-auto->'
style={{
borderTop: `1px solid ${colorTheme.seperator.value}`,
padding: `0 ${UtopiaTheme.layout.inspectorXPadding}px`,
alignItems: 'center',
gap: 8,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,6 @@ export const PaddingControl = React.memo(() => {
mode={modeToUse}
onCycleMode={onCycleMode}
values={values}
cycleModePosition='back'
/>
)
})
Expand Down
Loading

0 comments on commit ca05c44

Please sign in to comment.