Skip to content

Commit

Permalink
Fix/group children constraints section (#4377)
Browse files Browse the repository at this point in the history 
* checkpoint

* properly detect group constraints

* adding two todos, and patching a spotty behavior

* turning this on to fail some tests

* updating 'Fixed / Fill / Hug control' tests for the simplified layout section

* fix a test

* temporarily admit defeat

* fixing some describe('group children' tests

* fixing a test id

* temporarily using the old FS for Pins Section and Resize to Fit control

* fixing memoization issue

* updating perf tests

* sigh, also nerfing describe('inspector tests with real metadata',

* updating comment

* fix comment
  • Loading branch information
@balazsbajorics
balazsbajorics authored Oct 17, 2023
1 parent 8022034 commit 6a26341
Show file tree
Hide file tree
Showing 16 changed files with 578 additions and 664 deletions.
2 changes: 1 addition & 1 deletion editor/src/components/canvas/canvas-strategies/strategies/groups.spec.browser2.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4755,7 +4755,7 @@ describe('Inspector Pins section for group child', () => {

await selectComponentsForTest(editor, [fromString(`${GroupPath}/child-2`)])

await changeInspectorNumberControl(editor, 'position-top-number-input', '250')
await changeInspectorNumberControl(editor, 'frame-top-number-input', '250')

expect(groupDiv.style.width).toBe('200px')
expect(groupDiv.style.height).toBe('300px')
Expand Down
10 changes: 9 additions & 1 deletion editor/src/components/editor/actions/actions.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1489,6 +1489,7 @@ export const UPDATE_FNS = {
}
},
UNSET_PROPERTY: (action: UnsetProperty, editor: EditorModel): EditorModel => {
// TODO also queue group true up, just like for SET_PROP
let unsetPropFailedMessage: string | null = null
const updatedEditor = modifyUnderlyingElementForOpenFile(
action.element,
Expand Down Expand Up @@ -1545,7 +1546,14 @@ export const UPDATE_FNS = {
editor.allElementProps,
editor.projectContents,
)
if (isInvalidGroupState(maybeInvalidGroupState)) {
if (
isInvalidGroupState(maybeInvalidGroupState) &&
/**
* we want to exempt 'child-has-missing-pins' from this list, because SET_PROP maybe what the user is doing to _fix_ the situation highlighted by 'child-has-missing-pins'
* if 'child-has-missing-pins' prevents us from SET_PROP, that means we prevent ourselves from re-adding those missing props!
*/
maybeInvalidGroupState !== 'child-has-missing-pins'
) {
setPropFailedMessage = invalidGroupStateToString(maybeInvalidGroupState)
return element
}
Expand Down
105 changes: 61 additions & 44 deletions editor/src/components/inspector/common/inspector-end-to-end-tests.spec.browser2.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ import {
textFileContents,
unparsed,
} from '../../../core/shared/project-file-types'
import { expectSingleUndo2Saves, selectComponentsForTest } from '../../../utils/utils.test-utils'
import {
expectSingleUndo2Saves,
selectComponentsForTest,
setFeatureForBrowserTestsUseInDescribeBlockOnly,
wait,
} from '../../../utils/utils.test-utils'
import { contentsToTree } from '../../assets'
import { SubduedBorderRadiusControlTestId } from '../../canvas/controls/select-mode/subdued-border-radius-control'
import type { EditorRenderResult } from '../../canvas/ui-jsx.test-utils'
Expand Down Expand Up @@ -62,6 +67,7 @@ import type { InvalidGroupState } from '../../canvas/canvas-strategies/strategie
import { invalidGroupStateToString } from '../../canvas/canvas-strategies/strategies/group-helpers'
import selectEvent from 'react-select-event'
import { MixedPlaceholder } from '../../../uuiui/inputs/base-input'
import { cmdModifier } from '../../../utils/modifiers'

async function getControl(
controlTestId: string,
Expand Down Expand Up @@ -156,6 +162,7 @@ async function pressKeyTimes(
}

describe('inspector tests with real metadata', () => {
setFeatureForBrowserTestsUseInDescribeBlockOnly('Simplified Layout Section', false)
it('padding controls', async () => {
const renderResult = await renderTestEditorWithCode(
makeTestProjectCodeWithSnippet(`
Expand Down Expand Up @@ -3223,6 +3230,7 @@ describe('inspector tests with real metadata', () => {
})

describe('groups', () => {
setFeatureForBrowserTestsUseInDescribeBlockOnly('Simplified Layout Section', false)
function expectGroupToast(renderResult: EditorRenderResult, state: InvalidGroupState) {
const editorState = renderResult.getEditorState().editor
expect(editorState.toasts.length).toBe(1)
Expand Down Expand Up @@ -3436,6 +3444,7 @@ describe('inspector tests with real metadata', () => {
expectGroupToast(renderResult, 'child-has-percentage-pins')
})
describe('group children', () => {
setFeatureForBrowserTestsUseInDescribeBlockOnly('Simplified Layout Section', true)
const tests: {
name: string
input: string
Expand All @@ -3444,7 +3453,7 @@ describe('inspector tests with real metadata', () => {
want: string
}[] = [
{
name: 'set constraint for TLBR',
name: 'set constraint for Top, Height using the visual pin control',
input: `
<div
style={{ position: 'absolute', backgroundColor: '#FFFFFF' }}
Expand Down Expand Up @@ -3495,13 +3504,13 @@ describe('inspector tests with real metadata', () => {
data-uid='foo'
style={{
position: 'absolute',
top: 0,
left: 0,
width: 100,
backgroundColor: 'blue',
top: 0,
height: 100,
backgroundColor: 'blue'
}}
data-constraints={['top']}
data-constraints={['top', 'height']}
/>
<div
data-uid='bar'
Expand Down Expand Up @@ -3553,9 +3562,9 @@ describe('inspector tests with real metadata', () => {
selection: [EP.appendNewElementPath(TestScenePath, ['aaa', 'group', 'foo'])],
logic: async (renderResult) => {
const control = await renderResult.renderedDOM.findByTestId(
'group-child-controls-catcher-pin-top',
'group-child-controls-pin-centery-transparent',
)
await mouseClickAtPoint(control, { x: 1, y: 1 })
await mouseClickAtPoint(control, { x: 1, y: 1 }, { modifiers: cmdModifier })
},
want: `
<div
Expand All @@ -3569,11 +3578,11 @@ describe('inspector tests with real metadata', () => {
data-uid='foo'
style={{
position: 'absolute',
top: 0,
left: 0,
width: 100,
backgroundColor: 'blue',
top: 0,
height: 100,
backgroundColor: 'blue'
}}
/>
<div
Expand Down Expand Up @@ -3625,10 +3634,14 @@ describe('inspector tests with real metadata', () => {
`,
selection: [EP.appendNewElementPath(TestScenePath, ['aaa', 'group', 'foo'])],
logic: async (renderResult) => {
const control = await renderResult.renderedDOM.findByTestId(
const control1 = await renderResult.renderedDOM.findByTestId(
'group-child-controls-catcher-pin-top',
)
await mouseClickAtPoint(control, { x: 1, y: 1 })
await mouseClickAtPoint(control1, { x: 1, y: 1 })
const control2 = await renderResult.renderedDOM.findByTestId(
'group-child-controls-catcher-pin-left',
)
await mouseClickAtPoint(control2, { x: 1, y: 1 })
},
want: `
<div
Expand All @@ -3642,13 +3655,13 @@ describe('inspector tests with real metadata', () => {
data-uid='foo'
style={{
position: 'absolute',
backgroundColor: 'blue',
top: 0,
height: 100,
left: 0,
width: 100,
height: 100,
backgroundColor: 'blue'
}}
data-constraints={['left', 'width', 'top']}
data-constraints={['top', 'height', 'left', 'width']}
/>
<div
data-uid='bar'
Expand Down Expand Up @@ -3700,9 +3713,9 @@ describe('inspector tests with real metadata', () => {
selection: [EP.appendNewElementPath(TestScenePath, ['aaa', 'group', 'foo'])],
logic: async (renderResult) => {
const control = await renderResult.renderedDOM.findByTestId(
'group-child-controls-catcher-pin-top',
'group-child-controls-pin-centery-transparent',
)
await mouseClickAtPoint(control, { x: 1, y: 1 })
await mouseClickAtPoint(control, { x: 1, y: 1 }, { modifiers: cmdModifier })
},
want: `
<div
Expand All @@ -3716,11 +3729,11 @@ describe('inspector tests with real metadata', () => {
data-uid='foo'
style={{
position: 'absolute',
top: 0,
left: 0,
width: 100,
backgroundColor: 'blue',
top: 0,
height: 100,
backgroundColor: 'blue'
}}
data-constraints={['left', 'width']}
/>
Expand Down Expand Up @@ -3793,23 +3806,24 @@ describe('inspector tests with real metadata', () => {
data-uid='foo'
style={{
position: 'absolute',
top: 0,
left: 0,
width: 100,
backgroundColor: 'blue',
top: 0,
height: 100,
backgroundColor: 'blue'
}}
data-constraints={['left', 'width', 'top']}
data-constraints={['left', 'width', 'top', 'height']}
/>
<div
data-uid='bar'
style={{
position: 'absolute',
top: 200,
left: 200,
backgroundColor: 'red'
backgroundColor: 'red',
top: 200,
height: 0,
}}
data-constraints={['top']}
data-constraints={['top', 'height']}
/>
</Group>
</div>
Expand All @@ -3835,7 +3849,7 @@ describe('inspector tests with real metadata', () => {
height: 100,
backgroundColor: 'blue'
}}
data-constraints={['left', 'top', 'width']}
data-constraints={['left', 'width', 'top', 'height']}
/>
<div
data-uid='bar'
Expand Down Expand Up @@ -3871,30 +3885,31 @@ describe('inspector tests with real metadata', () => {
data-uid='foo'
style={{
position: 'absolute',
top: 0,
left: 0,
width: 100,
backgroundColor: 'blue',
top: 0,
height: 100,
backgroundColor: 'blue'
}}
data-constraints={['left', 'top', 'width']}
data-constraints={['left', 'width', 'top', 'height']}
/>
<div
data-uid='bar'
style={{
position: 'absolute',
top: 200,
left: 200,
backgroundColor: 'red'
backgroundColor: 'red',
top: 200,
height: 0,
}}
data-constraints={['top']}
data-constraints={['top', 'height']}
/>
</Group>
</div>
`,
},
{
name: 'unset constraint for TLBR, multiselect',
name: 'unset constraint for TH, multiselect',
input: `
<div
style={{ position: 'absolute', backgroundColor: '#FFFFFF' }}
Expand Down Expand Up @@ -3934,9 +3949,9 @@ describe('inspector tests with real metadata', () => {
],
logic: async (renderResult) => {
const control = await renderResult.renderedDOM.findByTestId(
'group-child-controls-catcher-pin-top',
'group-child-controls-pin-centery-transparent',
)
await mouseClickAtPoint(control, { x: 1, y: 1 })
await mouseClickAtPoint(control, { x: 1, y: 1 }, { modifiers: cmdModifier })
},
want: `
<div
Expand All @@ -3950,29 +3965,30 @@ describe('inspector tests with real metadata', () => {
data-uid='foo'
style={{
position: 'absolute',
top: 0,
left: 0,
width: 100,
backgroundColor: 'blue',
top: 0,
height: 100,
backgroundColor: 'blue'
}}
data-constraints={['left']}
/>
<div
data-uid='bar'
style={{
position: 'absolute',
top: 200,
left: 200,
backgroundColor: 'red'
backgroundColor: 'red',
top: 200,
height: 0,
}}
/>
</Group>
</div>
`,
},
{
name: 'set constraint for width/height',
name: 'set constraint for Left + Width',
input: `
<div
style={{ position: 'absolute', backgroundColor: '#FFFFFF' }}
Expand Down Expand Up @@ -4006,12 +4022,12 @@ describe('inspector tests with real metadata', () => {
`,
selection: [EP.appendNewElementPath(TestScenePath, ['aaa', 'group', 'foo'])],
logic: async (renderResult) => {
const control = document.getElementById('group-child-resize-width')
const control = document.getElementById('frame-child-constraint-width')
if (control == null) {
throw new Error('cannot find select')
}

await selectEvent.select(control, 'Fixed')
await selectEvent.select(control, 'Left')
},
want: `
<div
Expand All @@ -4026,12 +4042,12 @@ describe('inspector tests with real metadata', () => {
style={{
position: 'absolute',
top: 0,
height: 100,
backgroundColor: 'blue',
left: 0,
width: 100,
height: 100,
backgroundColor: 'blue'
}}
data-constraints={['width']}
data-constraints={['left', 'width']}
/>
<div
data-uid='bar'
Expand Down Expand Up @@ -4069,6 +4085,7 @@ describe('inspector tests with real metadata', () => {
})

describe('Inspector fields and code remain in sync', () => {
setFeatureForBrowserTestsUseInDescribeBlockOnly('Simplified Layout Section', false)
const propsToTest = [
{
stylePropKey: 'top',
Expand Down
Loading

0 comments on commit 6a26341

Please sign in to comment.