Skip to content

Commit

Permalink
Do not autolock root element when it is a leaf (#4230)
Browse files Browse the repository at this point in the history
* Do not autolock root element when it is a leaf

* Add tests

* Remove unnecessary code from test

* Making it more readable
  • Loading branch information
gbalint authored Sep 25, 2023
1 parent 12ae0ec commit 838a899
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 8 deletions.
7 changes: 6 additions & 1 deletion editor/src/components/editor/store/dispatch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,12 @@ function editorDispatchInner(
const priorSimpleLocks = storedState.unpatchedEditor.lockedElements.simpleLock
const updatedSimpleLocks = doNotUpdateLocks
? priorSimpleLocks
: updateSimpleLocks(storedState.unpatchedEditor.jsxMetadata, metadata, priorSimpleLocks)
: updateSimpleLocks(
storedState.unpatchedEditor.jsxMetadata,
metadata,
elementPathTree,
priorSimpleLocks,
)
if (result.unpatchedEditor.canvas.interactionSession != null) {
result = {
...result,
Expand Down
61 changes: 61 additions & 0 deletions editor/src/core/shared/element-locking.spec.browser2.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { renderTestEditorWithCode } from '../../components/canvas/ui-jsx.test-utils'
import * as EP from './element-path'

function testCode(snippet: string) {
return `
import * as React from 'react'
import { Scene, Storyboard } from 'utopia-api'
export var storyboard = (
<Storyboard data-uid='sb'>
<Scene
style={{
width: 700,
height: 759,
position: 'absolute',
left: 10,
top: 10,
}}
data-uid='sc'
>
<App data-uid='app' />
</Scene>
</Storyboard>
)
export var App = (props) => {
return (
<div
style={{
height: '100%',
width: '100%',
contain: 'layout',
}}
data-uid='app-root'
>
${snippet}
</div>
)
}`
}

describe('Auto-locking elements', () => {
it('Root element of component locked when it is not a leaf element', async () => {
const renderResult = await renderTestEditorWithCode(
testCode('<div>Hello</div>'),
'await-first-dom-report',
)

expect(renderResult.getEditorState().editor.lockedElements.simpleLock.map(EP.toString)).toEqual(
['sb/sc/app:app-root'],
)
})

it('Root element of component is not locked when it is a leaf element', async () => {
const renderResult = await renderTestEditorWithCode(testCode('Hello'), 'await-first-dom-report')

expect(renderResult.getEditorState().editor.lockedElements.simpleLock.map(EP.toString)).toEqual(
[],
)
})
})
21 changes: 14 additions & 7 deletions editor/src/core/shared/element-locking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,24 @@ import { MetadataUtils } from '../model/element-metadata-utils'
export function updateSimpleLocks(
priorMetadata: ElementInstanceMetadataMap,
newMetadata: ElementInstanceMetadataMap,
pathTree: ElementPathTrees,
currentSimpleLockedItems: Array<ElementPath>,
): Array<ElementPath> {
let result: Array<ElementPath> = [...currentSimpleLockedItems]
for (const [key, value] of Object.entries(newMetadata)) {
// This entry is the root element of an instance or a remix Outlet, and it isn't present in the previous metadata,
// which implies that it has been newly added.
if (
(EP.isRootElementOfInstance(value.elementPath) ||
MetadataUtils.isProbablyRemixOutlet(newMetadata, value.elementPath)) &&
!(key in priorMetadata)
) {
// The entry isn't present in the previous metadata, which implies that it has been newly added.
const isNewlyAdded = !(key in priorMetadata)

// You rarely want to select a root element of a component instance, except when it is a leaf element
const isNonLeafRootElement =
EP.isRootElementOfInstance(value.elementPath) &&
MetadataUtils.getChildrenPathsOrdered(newMetadata, pathTree, value.elementPath).length > 0

// Remix Outlet are rarely needed to be selected
const isRemixOutlet = MetadataUtils.isProbablyRemixOutlet(newMetadata, value.elementPath)

// This entry is a newly added non-leaf root element or a remix Outlet, it should be autolocked
if (isNewlyAdded && (isNonLeafRootElement || isRemixOutlet)) {
result.push(value.elementPath)
}
}
Expand Down

0 comments on commit 838a899

Please sign in to comment.