From 92886e1f40670018589d2c14dced119e47f8e6d1 Mon Sep 17 00:00:00 2001 From: alex Date: Tue, 3 Oct 2023 15:26:13 +0100 Subject: [PATCH] fix text in geo shapes not causing its container to grow (#2003) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We got things sliggghhhtly wrong in #1980. That diff was attempting to fix a bug where the text measurement element would refuse to go above the viewport size in safari. This was most obvious in the case where there was no fixed width on a text shape, and that diff fixed that case, but it was also happening when a fixed width text shape was wider than viewport - which wasn't covered by that fix. It turned out that that fix also introduced a bug where shapes would no longer grow along the y-axis - in part because the relationship between `width`, `maxWidth`, and `minWidth` is very confusing. The one-liner fix is to just use `max-content` instead of `fit-content` - that way, the div ignores the size of its container. But I also cleared up the API for text measurement to remove the `width` property entirely in favour of `maxWidth`. I think this makes things much clearer and as far as I can tell doesn't affect anything. Closes #1998 ### Change Type - [x] `patch` — Bug fix ### Test Plan 1. Create an arrow & geo shape with labels, plus a note and text shape 2. Try to break text measurement - overflow the bounds, make very wide text, experiment with fixed/auto-size text, etc. --- apps/examples/e2e/tests/test-text.spec.ts | 3 +-- packages/editor/editor.css | 2 +- .../src/lib/editor/managers/TextManager.ts | 22 +++++++------------ .../src/lib/defaultExternalContentHandlers.ts | 4 ++-- .../src/lib/shapes/arrow/ArrowShapeUtil.tsx | 6 ++--- .../src/lib/shapes/geo/GeoShapeUtil.tsx | 21 ++++++++---------- .../src/lib/shapes/note/NoteShapeUtil.tsx | 2 +- .../shapes/shared/default-shape-constants.ts | 1 - .../src/lib/shapes/text/TextShapeUtil.tsx | 4 ++-- packages/tldraw/src/test/TestEditor.ts | 10 ++++----- .../__snapshots__/getSvg.test.ts.snap | 6 ++--- 11 files changed, 34 insertions(+), 47 deletions(-) diff --git a/apps/examples/e2e/tests/test-text.spec.ts b/apps/examples/e2e/tests/test-text.spec.ts index 0177ed100..98a03f3f5 100644 --- a/apps/examples/e2e/tests/test-text.spec.ts +++ b/apps/examples/e2e/tests/test-text.spec.ts @@ -7,14 +7,13 @@ export function sleep(ms: number) { } const measureTextOptions = { - width: null, + maxWidth: null, fontFamily: 'var(--tl-font-draw)', fontSize: 24, lineHeight: 1.35, fontWeight: 'normal', fontStyle: 'normal', padding: '0px', - maxWidth: 'auto', } const measureTextSpansOptions = { diff --git a/packages/editor/editor.css b/packages/editor/editor.css index f7be2537f..65635af17 100644 --- a/packages/editor/editor.css +++ b/packages/editor/editor.css @@ -811,7 +811,7 @@ input, top: -9999px; right: -9999px; opacity: 0; - width: fit-content; + width: max-content; box-sizing: border-box; pointer-events: none; line-break: normal; diff --git a/packages/editor/src/lib/editor/managers/TextManager.ts b/packages/editor/src/lib/editor/managers/TextManager.ts index 3d24831f6..4d45aafea 100644 --- a/packages/editor/src/lib/editor/managers/TextManager.ts +++ b/packages/editor/src/lib/editor/managers/TextManager.ts @@ -40,7 +40,9 @@ const spaceCharacterRegex = /\s/ export class TextManager { constructor(public editor: Editor) {} - getTextElement() { + private textElement: HTMLDivElement | null = null + + private getTextElement() { const oldElm = document.querySelector('.tl-text-measure') oldElm?.remove() @@ -64,13 +66,12 @@ export class TextManager { fontSize: number lineHeight: number /** - * When width is a number, the text will be wrapped to that width. When - * width is null, the text will be measured without wrapping, but explicit - * line breaks and space are preserved. + * When maxWidth is a number, the text will be wrapped to that maxWidth. When maxWidth + * is null, the text will be measured without wrapping, but explicit line breaks and + * space are preserved. */ - width: null | number + maxWidth: null | number minWidth?: string - maxWidth: string padding: string } ): Box2dModel => { @@ -82,15 +83,8 @@ export class TextManager { elm.style.setProperty('font-weight', opts.fontWeight) elm.style.setProperty('font-size', opts.fontSize + 'px') elm.style.setProperty('line-height', opts.lineHeight * opts.fontSize + 'px') - if (opts.width === null) { - elm.style.setProperty('white-space', 'pre') - elm.style.setProperty('width', 'fit-content') - } else { - elm.style.setProperty('width', opts.width + 'px') - elm.style.setProperty('white-space', 'pre-wrap') - } + elm.style.setProperty('max-width', opts.maxWidth === null ? null : opts.maxWidth + 'px') elm.style.setProperty('min-width', opts.minWidth ?? null) - elm.style.setProperty('max-width', opts.maxWidth) elm.style.setProperty('padding', opts.padding) elm.textContent = normalizeTextForDom(textToMeasure) diff --git a/packages/tldraw/src/lib/defaultExternalContentHandlers.ts b/packages/tldraw/src/lib/defaultExternalContentHandlers.ts index b47e86263..c267379da 100644 --- a/packages/tldraw/src/lib/defaultExternalContentHandlers.ts +++ b/packages/tldraw/src/lib/defaultExternalContentHandlers.ts @@ -289,7 +289,7 @@ export function registerDefaultExternalContentHandlers( ...TEXT_PROPS, fontFamily: FONT_FAMILIES[defaultProps.font], fontSize: FONT_SIZES[defaultProps.size], - width: null, + maxWidth: null, }) const minWidth = Math.min( @@ -302,7 +302,7 @@ export function registerDefaultExternalContentHandlers( ...TEXT_PROPS, fontFamily: FONT_FAMILIES[defaultProps.font], fontSize: FONT_SIZES[defaultProps.size], - width: minWidth, + maxWidth: minWidth, }) w = shrunkSize.w h = shrunkSize.h diff --git a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx index 43b1cf69a..b69c7baaf 100644 --- a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx +++ b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx @@ -114,7 +114,7 @@ export class ArrowShapeUtil extends ShapeUtil { ...TEXT_PROPS, fontFamily: FONT_FAMILIES[shape.props.font], fontSize: ARROW_LABEL_FONT_SIZES[shape.props.size], - width: null, + maxWidth: null, }) let width = w @@ -129,7 +129,7 @@ export class ArrowShapeUtil extends ShapeUtil { ...TEXT_PROPS, fontFamily: FONT_FAMILIES[shape.props.font], fontSize: ARROW_LABEL_FONT_SIZES[shape.props.size], - width: width, + maxWidth: width, } ) @@ -146,7 +146,7 @@ export class ArrowShapeUtil extends ShapeUtil { ...TEXT_PROPS, fontFamily: FONT_FAMILIES[shape.props.font], fontSize: ARROW_LABEL_FONT_SIZES[shape.props.size], - width: width, + maxWidth: width, } ) diff --git a/packages/tldraw/src/lib/shapes/geo/GeoShapeUtil.tsx b/packages/tldraw/src/lib/shapes/geo/GeoShapeUtil.tsx index eb11c4405..9c98db5f6 100644 --- a/packages/tldraw/src/lib/shapes/geo/GeoShapeUtil.tsx +++ b/packages/tldraw/src/lib/shapes/geo/GeoShapeUtil.tsx @@ -1053,8 +1053,7 @@ function getLabelSize(editor: Editor, shape: TLGeoShape) { ...TEXT_PROPS, fontFamily: FONT_FAMILIES[shape.props.font], fontSize: LABEL_FONT_SIZES[shape.props.size], - width: null, - maxWidth: '100px', + maxWidth: 100, }) // TODO: Can I get these from somewhere? @@ -1069,17 +1068,15 @@ function getLabelSize(editor: Editor, shape: TLGeoShape) { ...TEXT_PROPS, fontFamily: FONT_FAMILIES[shape.props.font], fontSize: LABEL_FONT_SIZES[shape.props.size], - width: null, minWidth: minSize.w + 'px', - maxWidth: - Math.max( - // Guard because a DOM nodes can't be less 0 - 0, - // A 'w' width that we're setting as the min-width - Math.ceil(minSize.w + sizes[shape.props.size]), - // The actual text size - Math.ceil(shape.props.w - LABEL_PADDING * 2) - ) + 'px', + maxWidth: Math.max( + // Guard because a DOM nodes can't be less 0 + 0, + // A 'w' width that we're setting as the min-width + Math.ceil(minSize.w + sizes[shape.props.size]), + // The actual text size + Math.ceil(shape.props.w - LABEL_PADDING * 2) + ), }) return { diff --git a/packages/tldraw/src/lib/shapes/note/NoteShapeUtil.tsx b/packages/tldraw/src/lib/shapes/note/NoteShapeUtil.tsx index cc9df1973..f4c9f3fd2 100644 --- a/packages/tldraw/src/lib/shapes/note/NoteShapeUtil.tsx +++ b/packages/tldraw/src/lib/shapes/note/NoteShapeUtil.tsx @@ -194,7 +194,7 @@ function getGrowY(editor: Editor, shape: TLNoteShape, prevGrowY = 0) { ...TEXT_PROPS, fontFamily: FONT_FAMILIES[shape.props.font], fontSize: LABEL_FONT_SIZES[shape.props.size], - width: NOTE_SIZE - PADDING * 2, + maxWidth: NOTE_SIZE - PADDING * 2, }) const nextHeight = nextTextSize.h + PADDING * 2 diff --git a/packages/tldraw/src/lib/shapes/shared/default-shape-constants.ts b/packages/tldraw/src/lib/shapes/shared/default-shape-constants.ts index 78d66290d..f36aec472 100644 --- a/packages/tldraw/src/lib/shapes/shared/default-shape-constants.ts +++ b/packages/tldraw/src/lib/shapes/shared/default-shape-constants.ts @@ -7,7 +7,6 @@ export const TEXT_PROPS = { fontVariant: 'normal', fontStyle: 'normal', padding: '0px', - maxWidth: 'auto', } /** @public */ diff --git a/packages/tldraw/src/lib/shapes/text/TextShapeUtil.tsx b/packages/tldraw/src/lib/shapes/text/TextShapeUtil.tsx index dc0ad71d6..6d23f2a84 100644 --- a/packages/tldraw/src/lib/shapes/text/TextShapeUtil.tsx +++ b/packages/tldraw/src/lib/shapes/text/TextShapeUtil.tsx @@ -369,7 +369,7 @@ export class TextShapeUtil extends ShapeUtil { function getTextSize(editor: Editor, props: TLTextShape['props']) { const { font, text, autoSize, size, w } = props - const minWidth = 16 + const minWidth = autoSize ? 16 : Math.max(16, w) const fontSize = FONT_SIZES[size] const cw = autoSize @@ -381,7 +381,7 @@ function getTextSize(editor: Editor, props: TLTextShape['props']) { ...TEXT_PROPS, fontFamily: FONT_FAMILIES[font], fontSize: fontSize, - width: cw, + maxWidth: cw, }) // // If we're autosizing the measureText will essentially `Math.floor` diff --git a/packages/tldraw/src/test/TestEditor.ts b/packages/tldraw/src/test/TestEditor.ts index d5d953c71..e71c5db48 100644 --- a/packages/tldraw/src/test/TestEditor.ts +++ b/packages/tldraw/src/test/TestEditor.ts @@ -81,8 +81,7 @@ export class TestEditor extends Editor { fontFamily: string fontSize: number lineHeight: number - width: null | number - maxWidth: string + maxWidth: null | number } ): Box2dModel => { const breaks = textToMeasure.split('\n') @@ -95,9 +94,9 @@ export class TestEditor extends Editor { return { x: 0, y: 0, - w: opts.width === null ? w : Math.max(w, opts.width), + w: opts.maxWidth === null ? w : Math.max(w, opts.maxWidth), h: - (opts.width === null ? breaks.length : Math.ceil(w % opts.width) + breaks.length) * + (opts.maxWidth === null ? breaks.length : Math.ceil(w % opts.maxWidth) + breaks.length) * opts.fontSize, } } @@ -105,9 +104,8 @@ export class TestEditor extends Editor { this.textMeasure.measureTextSpans = (textToMeasure, opts) => { const box = this.textMeasure.measureText(textToMeasure, { ...opts, - width: opts.width, + maxWidth: opts.width, padding: `${opts.padding}px`, - maxWidth: 'auto', }) return [{ box, text: textToMeasure }] } diff --git a/packages/tldraw/src/test/commands/__snapshots__/getSvg.test.ts.snap b/packages/tldraw/src/test/commands/__snapshots__/getSvg.test.ts.snap index 4e77da6c4..6de62d0d3 100644 --- a/packages/tldraw/src/test/commands/__snapshots__/getSvg.test.ts.snap +++ b/packages/tldraw/src/test/commands/__snapshots__/getSvg.test.ts.snap @@ -87,7 +87,7 @@ exports[`Matches a snapshot: Basic SVG 1`] = ` > Hello world @@ -127,7 +127,7 @@ exports[`Matches a snapshot: Basic SVG 1`] = ` Hello world