Skip to content

Commit

Permalink
fix hot reload text measurement bug (tldraw#5125)
Browse files Browse the repository at this point in the history
fixes
tldraw#5047 (comment)

the problem was that during hot reload react was rerendering the
container elem but the editor wasn't recreated at the same, so the
sneaky div that the text measurement manager adds was being disconnected
from the DOM.

### Change type

- [x] `bugfix`
  • Loading branch information
ds300 authored Dec 16, 2024
1 parent f59352b commit ed2113d
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 11 deletions.
4 changes: 2 additions & 2 deletions packages/editor/api-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -2528,9 +2528,9 @@ export type TestEnvironment = 'development' | 'production';
export class TextManager {
constructor(editor: Editor);
// (undocumented)
baseElm: HTMLDivElement;
// (undocumented)
editor: Editor;
// (undocumented)
getBaseElm(): HTMLDivElement;
measureElementTextNodeSpans(element: HTMLElement, { shouldTruncateToFirstLine }?: {
shouldTruncateToFirstLine?: boolean;
}): {
Expand Down
2 changes: 1 addition & 1 deletion packages/editor/src/lib/TldrawEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ function TldrawEditorWithReadyStore({
) : (
<EditorProvider editor={editor}>
<Layout onMount={onMount}>
{children ?? (Canvas ? <Canvas /> : null)}
{children ?? (Canvas ? <Canvas key={editor.contextId} /> : null)}
<Watermark />
</Layout>
</EditorProvider>
Expand Down
26 changes: 18 additions & 8 deletions packages/editor/src/lib/editor/managers/TextManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,14 @@ const spaceCharacterRegex = /\s/

/** @public */
export class TextManager {
baseElm: HTMLDivElement
private _baseElm: HTMLDivElement | null = null

constructor(public editor: Editor) {
// In some hot reloading scenarios the base element can be removed from the dom.
// So every time we use it we need to check if it's still connected and create a new one if not.
getBaseElm() {
if (this._baseElm?.isConnected) {
return this._baseElm
}
const container = this.editor.getContainer()

const elm = document.createElement('div')
Expand All @@ -49,9 +54,12 @@ export class TextManager {
elm.tabIndex = -1
container.appendChild(elm)

this.baseElm = elm
return (this._baseElm = elm)
}

constructor(public editor: Editor) {
editor.disposables.add(() => {
elm.remove()
this._baseElm?.remove()
})
}

Expand All @@ -75,8 +83,9 @@ export class TextManager {
}
): BoxModel & { scrollWidth: number } {
// Duplicate our base element; we don't need to clone deep
const elm = this.baseElm?.cloneNode() as HTMLDivElement
this.baseElm.insertAdjacentElement('afterend', elm)
const baseElem = this.getBaseElm()
const elm = baseElem.cloneNode() as HTMLDivElement
baseElem.insertAdjacentElement('afterend', elm)

elm.setAttribute('dir', 'auto')
// N.B. This property, while discouraged ("intended for Document Type Definition (DTD) designers")
Expand Down Expand Up @@ -223,8 +232,9 @@ export class TextManager {
): { text: string; box: BoxModel }[] {
if (textToMeasure === '') return []

const elm = this.baseElm?.cloneNode() as HTMLDivElement
this.baseElm.insertAdjacentElement('afterend', elm)
const baseElem = this.getBaseElm()
const elm = baseElem.cloneNode() as HTMLDivElement
baseElem.insertAdjacentElement('afterend', elm)

const elementWidth = Math.ceil(opts.width - opts.padding * 2)
elm.setAttribute('dir', 'auto')
Expand Down

0 comments on commit ed2113d

Please sign in to comment.