Skip to content

Commit

Permalink
Modifiers for reparenting and convert to absolute strategies (#6054)
Browse files Browse the repository at this point in the history
**Description:**
To make our canvas interactions more predictable I put reparenting and
conversion to absolute behind modifier keys:
- cmd is needed for reparenting
- ctrl is needed for conversion to absolute

**Notes:**
- Nearly 70 tests were broken (nearly all reparenting and
convert-to-absolute tests). In most of the cases I just had to add the
necessary modifiers to make the tests work
- Insertion relies on reparenting, I needed to modify the draw-to-insert
strategy too
- cmd disables snapping, so now it is not possible to have snapping
during reparenting (I needed to disable the tests for this)
- space forces convert to absolute, this is in partial conflict with
ctrl, but they are not really the same: without ctrl convert-to-absolute
strategies are disabled, but even with ctrl other strategies are still
possible, while space really forces convert-to-absolute strategies
- when you have a selection, cmd mouse down on a descendant element will
change the selection to the descendant. If you don't want that, you need
to press cmd after the mouse down.
- with ctrl, you always need to press it after mouse down, otherwise the
context menu will open
- earlier cmd had a special meaning in case of reparenting: it allowed
the element to be reparented into a parent which is smaller than the
element itself. This distinction doesn't exist anymore, because without
pressing cmd there is no reparent at all

**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 #6047
  • Loading branch information
gbalint authored and liady committed Dec 13, 2024
1 parent aada52f commit 8dd1d07
Show file tree
Hide file tree
Showing 14 changed files with 468 additions and 148 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,62 @@ async function dragElement(
const defaultMouseDownOffset = windowPoint({ x: 20, y: 20 })

describe('Unified Reparent Fitness Function Tests', () => {
it('if cmd is not pressed no reparent strategies have nonzero fitness', async () => {
const renderResult = await renderTestEditorWithCode(
makeTestProjectCodeWithSnippet(`
<div
style={{
backgroundColor: 'white',
position: 'absolute',
width: '100%',
height: '100%',
}}
data-uid='aaa'
>
<div
style={{
backgroundColor: '#aaaaaa33',
position: 'absolute',
left: 40,
top: 40,
width: 100,
height: 100,
}}
data-uid='bbb'
>
<div
style={{
backgroundColor: '#aaaaaa33',
position: 'absolute',
left: 0,
top: 0,
width: 200,
height: 200,
}}
data-uid='ccc'
data-testid='ccc'
/>
</div>
</div>
`),
'await-first-dom-report',
)

const dragDelta = windowPoint({ x: 120, y: 0 })

const targetPath = EP.appendNewElementPath(TestScenePath, ['aaa', 'bbb', 'ccc'])
await renderResult.dispatch([selectComponents([targetPath], false)], false)
await dragElement(renderResult, 'ccc', defaultMouseDownOffset, dragDelta, emptyModifiers, false)

const strategies = renderResult.getEditorState().strategyState.sortedApplicableStrategies

if (strategies == null) {
// here for type assertion
throw new Error('`strategies` should not be null')
}
expect(strategies[0].strategy.id.includes('REPARENT')).toBeFalsy()
})

it('if an element is larger than its parent, we still allow reparent to its grandparent, if the reparenting starts from the area of the original parent', async () => {
const renderResult = await renderTestEditorWithCode(
makeTestProjectCodeWithSnippet(`
Expand Down Expand Up @@ -110,7 +166,7 @@ describe('Unified Reparent Fitness Function Tests', () => {

const targetPath = EP.appendNewElementPath(TestScenePath, ['aaa', 'bbb', 'ccc'])
await renderResult.dispatch([selectComponents([targetPath], false)], false)
await dragElement(renderResult, 'ccc', defaultMouseDownOffset, dragDelta, emptyModifiers, true)
await dragElement(renderResult, 'ccc', defaultMouseDownOffset, dragDelta, cmdModifier, true)

await renderResult.getDispatchFollowUpActionsFinished()

Expand Down Expand Up @@ -198,7 +254,7 @@ describe('Unified Reparent Fitness Function Tests', () => {
const targetPath = EP.appendNewElementPath(TestScenePath, ['aaa', 'bbb', 'ccc'])
await renderResult.dispatch([selectComponents([targetPath], false)], false)

await dragElement(renderResult, 'ccc', defaultMouseDownOffset, dragDelta, emptyModifiers, true)
await dragElement(renderResult, 'ccc', defaultMouseDownOffset, dragDelta, cmdModifier, true)

await renderResult.getDispatchFollowUpActionsFinished()

Expand Down Expand Up @@ -291,7 +347,7 @@ describe('Unified Reparent Fitness Function Tests', () => {
'draggedElement',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
true,
)

Expand Down Expand Up @@ -486,7 +542,7 @@ describe('Unified Reparent Fitness Function Tests', () => {
'draggedElement',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
true,
)

Expand Down Expand Up @@ -582,7 +638,7 @@ describe('Unified Reparent Fitness Function Tests', () => {
'draggedElement',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
true,
)

Expand Down Expand Up @@ -949,7 +1005,7 @@ describe('Target parents with flow layout', () => {
'draggedElement',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
true,
)

Expand Down Expand Up @@ -1092,7 +1148,7 @@ describe('Target parents with flow layout', () => {
'draggedElement',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
true,
)

Expand Down Expand Up @@ -1200,7 +1256,7 @@ describe('Target parents with flow layout', () => {
'draggedElement',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
true,
)

Expand Down Expand Up @@ -1297,7 +1353,7 @@ describe('Target parents with flow layout', () => {
'draggedElement',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
true,
() => pressKey('2', { modifiers: cmdModifier }), // Switch to flow reparenting strategy
)
Expand Down Expand Up @@ -1423,7 +1479,7 @@ describe('Target parents with flow layout', () => {
'draggedElement',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
true,
() => pressKey('2', { modifiers: cmdModifier }), // Switch to flow reparenting strategy
)
Expand Down Expand Up @@ -1918,7 +1974,8 @@ describe('Target parent filtering', () => {
)
})

it('Dragging without cmd with cursor over the smaller element prevents reparenting into the larger element', async () => {
// now we need cmd for any kind of reparent, so this test is outdated
xit('Dragging without cmd with cursor over the smaller element prevents reparenting into the larger element', async () => {
const renderResult = await renderTestEditorWithCode(startingCode, 'await-first-dom-report')

const targetPath = EP.elementPath([[BakedInStoryboardUID, TestSceneUID, 'dragme']])
Expand All @@ -1929,7 +1986,7 @@ describe('Target parent filtering', () => {
// drag to the left so that the cursor is over the larger element
const dragDelta = windowPoint({ x: -10, y: -10 })

await dragElement(renderResult, 'dragme', mouseDownOffset, dragDelta, emptyModifiers, true)
await dragElement(renderResult, 'dragme', mouseDownOffset, dragDelta, cmdModifier, true)

await renderResult.getDispatchFollowUpActionsFinished()

Expand Down Expand Up @@ -2272,7 +2329,7 @@ describe('Reparent indicators', () => {
'seconddiv',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
false,
)

Expand Down Expand Up @@ -2318,7 +2375,7 @@ describe('Reparent indicators', () => {
'seconddiv',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
false,
)

Expand Down Expand Up @@ -2361,7 +2418,7 @@ describe('Reparent indicators', () => {
'seconddiv',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
false,
)

Expand Down Expand Up @@ -2404,7 +2461,7 @@ describe('Reparent indicators', () => {
'seconddiv',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
false,
)

Expand Down Expand Up @@ -2448,7 +2505,7 @@ describe('Reparent indicators', () => {
'absolutechild',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
false,
)

Expand Down Expand Up @@ -2492,7 +2549,7 @@ describe('Reparent indicators', () => {
'absolutechild',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
false,
)

Expand Down Expand Up @@ -2536,7 +2593,7 @@ describe('Reparent indicators', () => {
'absolutechild',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
false,
)

Expand Down Expand Up @@ -2580,7 +2637,7 @@ describe('Reparent indicators', () => {
'absolutechild',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
false,
)

Expand Down Expand Up @@ -2660,11 +2717,11 @@ describe('Reparent indicators', () => {
'absolutechild',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
false,
)

await pressKey('2') // Switch to flow reparenting strategy
await pressKey('2', { modifiers: cmdModifier }) // Switch to flow reparenting strategy

await renderResult.getDispatchFollowUpActionsFinished()

Expand Down Expand Up @@ -2758,11 +2815,11 @@ describe('Reparent indicators', () => {
'absolutechild',
defaultMouseDownOffset,
dragDelta,
emptyModifiers,
cmdModifier,
false,
)

await pressKey('2') // Switch to flow reparenting strategy
await pressKey('2', { modifiers: cmdModifier }) // Switch to flow reparenting strategy

await renderResult.getDispatchFollowUpActionsFinished()

Expand Down
Loading

0 comments on commit 8dd1d07

Please sign in to comment.