Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix subscribing mutation/resize observers during a build error #6344

Merged
merged 10 commits into from
Sep 11, 2024
13 changes: 9 additions & 4 deletions editor/src/components/canvas/canvas-component-entry.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ import type {
} from './ui-jsx-canvas'
import { DomWalkerInvalidatePathsCtxAtom, UiJsxCanvas, pickUiJsxCanvasProps } from './ui-jsx-canvas'

interface CanvasComponentEntryProps {}
export const CanvasContainerOuterId = 'canvas-container-outer'

interface CanvasComponentEntryProps {
shouldRenderCanvas: boolean
}

export const CanvasComponentEntry = React.memo((props: CanvasComponentEntryProps) => {
const canvasStore = React.useContext(CanvasStateContext)
Expand Down Expand Up @@ -85,22 +89,23 @@ const CanvasComponentEntryInner = React.memo((props: CanvasComponentEntryProps)
<>
{when(canvasProps == null, <CanvasLoadingScreen />)}
<div
id='canvas-container-outer'
id={CanvasContainerOuterId}
ref={containerRef}
style={{
position: 'absolute',
transition: canvasScrollAnimation ? 'transform 0.3s ease-in-out' : 'initial',
transform: 'translate3d(0px, 0px, 0px)',
}}
>
{canvasProps == null ? null : (
{props.shouldRenderCanvas && canvasProps != null ? (
<CanvasInner
canvasProps={canvasProps}
onRuntimeError={onRuntimeError}
localClearRuntimeErrors={localClearRuntimeErrors}
/>
)}
) : null}
</div>
,
</>
)
})
Expand Down
21 changes: 10 additions & 11 deletions editor/src/components/canvas/canvas-wrapper-component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,16 @@ export const CanvasWrapperComponent = React.memo(() => {
// ^ prevents Monaco from pushing the inspector out
}}
>
{fatalErrors.length === 0 && !safeMode ? (
<EditorCanvas
userState={userState}
editor={editorState}
derived={derivedState}
model={createCanvasModelKILLME(editorState, derivedState)}
builtinDependencies={builtinDependencies}
updateCanvasSize={updateCanvasSize}
dispatch={dispatch}
/>
) : null}
<EditorCanvas
userState={userState}
editor={editorState}
derived={derivedState}
model={createCanvasModelKILLME(editorState, derivedState)}
builtinDependencies={builtinDependencies}
updateCanvasSize={updateCanvasSize}
dispatch={dispatch}
shouldRenderCanvas={fatalErrors.length === 0 && !safeMode}
/>

<FlexRow
style={{
Expand Down
9 changes: 5 additions & 4 deletions editor/src/components/canvas/dom-walker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ import {
} from '../inspector/common/css-utils'
import { camelCaseToDashed } from '../../core/shared/string-utils'
import type { UtopiaStoreAPI } from '../editor/store/store-hook'
import { UTOPIA_SCENE_ID_KEY } from '../../core/model/utopia-constants'
import { CanvasContainerID } from './canvas-types'
import { UTOPIA_SCENE_ID_KEY, UTOPIA_UID_KEY } from '../../core/model/utopia-constants'
import { emptySet } from '../../core/shared/set-utils'
import type { PathWithString } from '../../core/shared/uid-utils'
import { getDeepestPathOnDomElement, getPathStringsOnDomElement } from '../../core/shared/uid-utils'
Expand All @@ -72,6 +71,7 @@ import { pick } from '../../core/shared/object-utils'
import { getFlexAlignment, getFlexJustifyContent, MaxContent } from '../inspector/inspector-common'
import type { EditorDispatch } from '../editor/action-types'
import { runDOMWalker } from '../editor/actions/action-creators'
import { CanvasContainerOuterId } from './canvas-component-entry'

export const ResizeObserver =
window.ResizeObserver ?? ResizeObserverSyntheticDefault.default ?? ResizeObserverSyntheticDefault
Expand Down Expand Up @@ -291,14 +291,15 @@ export function resubscribeObservers(domWalkerMutableState: {
mutationObserver: MutationObserver
resizeObserver: ResizeObserver
}) {
const canvasRootContainer = document.getElementById(CanvasContainerID)
const canvasRootContainer = document.getElementById(CanvasContainerOuterId)

if (
ObserversAvailable &&
canvasRootContainer != null &&
domWalkerMutableState.resizeObserver != null &&
domWalkerMutableState.mutationObserver != null
) {
document.querySelectorAll(`#${CanvasContainerID} *`).forEach((elem) => {
document.querySelectorAll(`#${CanvasContainerOuterId} [${UTOPIA_UID_KEY}]`).forEach((elem) => {
domWalkerMutableState.resizeObserver.observe(elem)
})
domWalkerMutableState.mutationObserver.observe(canvasRootContainer, MutationObserverConfig)
Expand Down
5 changes: 5 additions & 0 deletions editor/src/components/canvas/ui-jsx.test-utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,8 @@ export function optOutFromCheckFileTimestamps() {
})
}

let prevDomWalkerMutableState: DomWalkerMutableStateData | null = null

export async function renderTestEditorWithModel(
rawModel: PersistentModel,
awaitFirstDomReport: 'await-first-dom-report' | 'dont-await-first-dom-report',
Expand Down Expand Up @@ -600,7 +602,10 @@ export async function renderTestEditorWithModel(
patchedStoreFromFullStore(initialEditorStore, 'canvas-store'),
)

prevDomWalkerMutableState?.resizeObserver.disconnect()
prevDomWalkerMutableState?.mutationObserver.disconnect()
const domWalkerMutableState = createDomWalkerMutableState(canvasStoreHook, asyncTestDispatch)
prevDomWalkerMutableState = domWalkerMutableState

const lowPriorityStoreHook: UtopiaStoreAPI = createStoresAndState(
patchedStoreFromFullStore(initialEditorStore, 'low-priority-store'),
Expand Down
16 changes: 10 additions & 6 deletions editor/src/templates/editor-canvas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,7 @@ interface EditorCanvasProps {
builtinDependencies: BuiltInDependencies
dispatch: EditorDispatch
updateCanvasSize: (newValueOrUpdater: Size | ((oldValue: Size) => Size)) => void
shouldRenderCanvas: boolean
}

export class EditorCanvas extends React.Component<EditorCanvasProps> {
Expand Down Expand Up @@ -855,6 +856,10 @@ export class EditorCanvas extends React.Component<EditorCanvasProps> {
// we round the offset here, so all layers, the canvas, and controls use the same rounded value for positioning
// instead of letting Chrome do it, because that results in funky artifacts (e.g. while scrolling, the layers don't "jump" pixels at the same time)

if (!this.props.shouldRenderCanvas) {
return <CanvasComponentEntry shouldRenderCanvas={false} />
}

const nodeConnectorsDiv = createNodeConnectorsDiv(
this.props.model.canvasOffset,
this.props.model.scale,
Expand All @@ -871,10 +876,9 @@ export class EditorCanvas extends React.Component<EditorCanvasProps> {

const canvasIsLive = isLiveMode(this.props.editor.mode)

const canvasControls = React.createElement(NewCanvasControls, {
windowToCanvasPosition: this.getPosition,
cursor,
})
const canvasControls = (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so I am only reading this code and haven't tried it yet, but doesn't this mean this will become another div that is mounted a bit higher in the tree?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was always there. Balint has just updated this to use JSX syntax rather than React.createElement

<NewCanvasControls windowToCanvasPosition={this.getPosition} cursor={cursor} />
)

const canvasLiveEditingStyle = canvasIsLive
? UtopiaStyles.canvas.live
Expand Down Expand Up @@ -1070,9 +1074,9 @@ export class EditorCanvas extends React.Component<EditorCanvasProps> {
},
},
nodeConnectorsDiv,
React.createElement(CanvasComponentEntry, {}),
<CanvasComponentEntry shouldRenderCanvas={true} />,
canvasControls,
React.createElement(CursorComponent, {}),
<CursorComponent />,
<EditorCommon mouseDown={this.handleMouseDown} />,
)
}
Expand Down
Loading