Skip to content

Commit

Permalink
Move grid placement section below layout, use self alignment icons fo…
Browse files Browse the repository at this point in the history
…r grid/flex children (#6502)

**Problem:**

1. The grid placement subsection should be just below the layout
section, since they are all related to the positioning of the selected
element
2. Grid and Flex children should use the `[align|justify]Self` icons for
the alignment buttons instead of the non-self ones.

**Fix:**

Do that.

| Before | After |
|---------|--------------|
| <img width="276" alt="Screenshot 2024-10-09 at 11 48 24"
src="https://github.com/user-attachments/assets/384dd748-17c6-4553-9d38-11b6c9dd1c8f">
|<img width="276" alt="Screenshot 2024-10-09 at 11 48 06"
src="https://github.com/user-attachments/assets/31a3beb2-5201-4980-b9aa-1e19382d53eb">
|


Fixes #6501
  • Loading branch information
ruggi authored Oct 9, 2024
1 parent 9ba3fe1 commit 29c7829
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 27 deletions.
40 changes: 27 additions & 13 deletions editor/src/components/inspector/alignment-buttons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const AlignmentButtons = React.memo(() => {

const disableDistribute = selectedViews.current.length < 3

const activeAlignments = useActiveAlignments()
const { activeAlignments, allSelectedViewsAreFlexOrGridChildren } = useActiveAlignments()
const hasActiveAlignments = React.useMemo(() => {
return (
activeAlignments.bottom ||
Expand Down Expand Up @@ -174,6 +174,9 @@ export const AlignmentButtons = React.memo(() => {
const disableJustify = useDisableAlignment(selectedViews.current, 'horizontal')
const disableAlign = useDisableAlignment(selectedViews.current, 'vertical')

const justifyIconPrefix = allSelectedViewsAreFlexOrGridChildren ? 'justifySelf' : 'justify'
const alignIconPrefix = allSelectedViewsAreFlexOrGridChildren ? 'alignSelf' : 'align'

return (
<UIGridRow padded={false} variant='<--1fr--><--1fr-->|22px|'>
<OptionChainControl
Expand All @@ -187,19 +190,19 @@ export const AlignmentButtons = React.memo(() => {
options={[
{
value: 'left',
icon: { category: 'inspector', type: 'justify-start' },
icon: { category: 'inspector', type: `${justifyIconPrefix}-start` },
forceCallOnSubmitValue: true,
disabled: disableJustify,
},
{
value: 'hcenter',
icon: { category: 'inspector', type: 'justify-center' },
icon: { category: 'inspector', type: `${justifyIconPrefix}-center` },
forceCallOnSubmitValue: true,
disabled: disableJustify,
},
{
value: 'right',
icon: { category: 'inspector', type: 'justify-end' },
icon: { category: 'inspector', type: `${justifyIconPrefix}-end` },
forceCallOnSubmitValue: true,
disabled: disableJustify,
},
Expand All @@ -216,19 +219,19 @@ export const AlignmentButtons = React.memo(() => {
options={[
{
value: 'top',
icon: { category: 'inspector', type: 'align-start' },
icon: { category: 'inspector', type: `${alignIconPrefix}-start` },
forceCallOnSubmitValue: true,
disabled: disableAlign,
},
{
value: 'vcenter',
icon: { category: 'inspector', type: 'align-center' },
icon: { category: 'inspector', type: `${alignIconPrefix}-center` },
forceCallOnSubmitValue: true,
disabled: disableAlign,
},
{
value: 'bottom',
icon: { category: 'inspector', type: 'align-end' },
icon: { category: 'inspector', type: `${alignIconPrefix}-end` },
forceCallOnSubmitValue: true,
disabled: disableAlign,
},
Expand All @@ -241,7 +244,10 @@ export const AlignmentButtons = React.memo(() => {
})
AlignmentButtons.displayName = 'AlignmentButtons'

function useActiveAlignments(): ActiveAlignments {
function useActiveAlignments(): {
activeAlignments: ActiveAlignments
allSelectedViewsAreFlexOrGridChildren: boolean
} {
const selectedViews = useRefEditorState((store) => store.editor.selectedViews)

const jsxMetadata = useEditorState(
Expand All @@ -250,16 +256,19 @@ function useActiveAlignments(): ActiveAlignments {
'useActiveAlignments jsxMetadata',
)

const allSelectedViewsAreFlexOrGridChildren = React.useMemo(
() => selectedViews.current.every((path) => MetadataUtils.isFlexOrGridChild(jsxMetadata, path)),
[selectedViews, jsxMetadata],
)

const isActive = React.useCallback(
(alignment: Alignment) => {
if (selectedViews.current.length === 0) {
return false
}

// Only flex/grid children can have active alignments, because they would mean nothing for flow elements.
if (
!selectedViews.current.every((view) => MetadataUtils.isFlexOrGridChild(jsxMetadata, view))
) {
if (!allSelectedViewsAreFlexOrGridChildren) {
return false
}

Expand Down Expand Up @@ -321,10 +330,10 @@ function useActiveAlignments(): ActiveAlignments {
}
})
},
[jsxMetadata, selectedViews],
[jsxMetadata, selectedViews, allSelectedViewsAreFlexOrGridChildren],
)

return React.useMemo(() => {
const activeAlignments = React.useMemo(() => {
return {
left: isActive('left'),
hcenter: isActive('hcenter'),
Expand All @@ -334,6 +343,11 @@ function useActiveAlignments(): ActiveAlignments {
bottom: isActive('bottom'),
}
}, [isActive])

return {
activeAlignments: activeAlignments,
allSelectedViewsAreFlexOrGridChildren: allSelectedViewsAreFlexOrGridChildren,
}
}

type UnsetAlignment = {
Expand Down
10 changes: 0 additions & 10 deletions editor/src/components/inspector/inspector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ 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'
import { isTailwindEnabled } from '../../core/tailwind/tailwind-compilation'

Expand Down Expand Up @@ -296,14 +295,6 @@ export const Inspector = React.memo<InspectorProps>((props: InspectorProps) => {

const shouldShowSimplifiedLayoutSection = inspectorPreferences.includes('layout')

const shouldShowGridCellSection = useEditorState(
Substores.metadata,
(store) =>
store.editor.selectedViews.length === 1 &&
MetadataUtils.isGridCell(store.editor.jsxMetadata, store.editor.selectedViews[0]),
'Inspector shouldShowGridCellSection',
)

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

Expand Down Expand Up @@ -373,7 +364,6 @@ export const Inspector = React.memo<InspectorProps>((props: InspectorProps) => {
<ConstraintsSection />
</>,
)}
{when(shouldShowGridCellSection, <GridPlacementSubsection />)}
{when(shouldShowFlexSection, <FlexSection />)}
{when(
multiselectedContract === 'frame' || multiselectedContract === 'wrapper-div',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { FrameUpdatingLayoutSection } from './frame-updating-layout-section'
import { MetadataUtils } from '../../../../../core/model/element-metadata-utils'
import { getInspectorPreferencesForTargets } from '../../../../../core/property-controls/property-controls-utils'
import { AlignmentButtons } from '../../../alignment-buttons'
import { GridPlacementSubsection } from '../../style-section/container-subsection/grid-cell-subsection'

export const SimplifiedLayoutSubsection = React.memo(() => {
const selectedElementContract = useEditorState(
Expand Down Expand Up @@ -56,6 +57,14 @@ export const SimplifiedLayoutSubsection = React.memo(() => {

const shouldShowAlignmentButtons = !isCodeElement && inspectorPreferences.includes('layout')

const shouldShowGridCellSection = useEditorState(
Substores.metadata,
(store) =>
store.editor.selectedViews.length === 1 &&
MetadataUtils.isGridCell(store.editor.jsxMetadata, store.editor.selectedViews[0]),
'Inspector shouldShowGridCellSection',
)

return (
<FlexColumn>
<FlexRow
Expand Down Expand Up @@ -90,6 +99,7 @@ export const SimplifiedLayoutSubsection = React.memo(() => {
</>,
)}
</FlexColumn>
{when(shouldShowGridCellSection, <GridPlacementSubsection />)}
</FlexColumn>
)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ Array [
"/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/Inspector/Symbol(react.memo)()",
"/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/Inspector/Symbol(react.memo)(ConstraintsSection)",
"/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/Inspector/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/Inspector/Symbol(react.memo)(GridPlacementSubsection)",
"/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/Inspector/Symbol(react.memo)()",
"/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/Inspector/Symbol(react.memo)(StyleSection)",
"/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/Inspector/Symbol(react.memo)()",
Expand Down Expand Up @@ -901,7 +900,6 @@ Array [
"/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/Inspector/Symbol(react.memo)()",
"/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/Inspector/Symbol(react.memo)(ConstraintsSection)",
"/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/Inspector/UtopiaSpiedExoticType(Symbol(react.fragment))",
"/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/Inspector/Symbol(react.memo)(GridPlacementSubsection)",
"/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/Inspector/Symbol(react.memo)()",
"/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/Inspector/Symbol(react.memo)(StyleSection)",
"/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/Inspector/Symbol(react.memo)()",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ describe('React Render Count Tests -', () => {

const renderCountAfter = renderResult.getNumberOfRenders()
// if this breaks, GREAT NEWS but update the test please :)
expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`651`)
expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`650`)
expect(renderResult.getRenderInfo()).toMatchSnapshot()
})

Expand Down Expand Up @@ -249,7 +249,7 @@ describe('React Render Count Tests -', () => {

const renderCountAfter = renderResult.getNumberOfRenders()
// if this breaks, GREAT NEWS but update the test please :)
expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`776`)
expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`775`)
expect(renderResult.getRenderInfo()).toMatchSnapshot()
})
})

0 comments on commit 29c7829

Please sign in to comment.