fix text in geo shapes not causing its container to grow (#2003)

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.
This commit is contained in:
alex 2023-10-03 15:26:13 +01:00 committed by GitHub
parent f414548347
commit 92886e1f40
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 34 additions and 47 deletions

View file

@ -7,14 +7,13 @@ export function sleep(ms: number) {
} }
const measureTextOptions = { const measureTextOptions = {
width: null, maxWidth: null,
fontFamily: 'var(--tl-font-draw)', fontFamily: 'var(--tl-font-draw)',
fontSize: 24, fontSize: 24,
lineHeight: 1.35, lineHeight: 1.35,
fontWeight: 'normal', fontWeight: 'normal',
fontStyle: 'normal', fontStyle: 'normal',
padding: '0px', padding: '0px',
maxWidth: 'auto',
} }
const measureTextSpansOptions = { const measureTextSpansOptions = {

View file

@ -811,7 +811,7 @@ input,
top: -9999px; top: -9999px;
right: -9999px; right: -9999px;
opacity: 0; opacity: 0;
width: fit-content; width: max-content;
box-sizing: border-box; box-sizing: border-box;
pointer-events: none; pointer-events: none;
line-break: normal; line-break: normal;

View file

@ -40,7 +40,9 @@ const spaceCharacterRegex = /\s/
export class TextManager { export class TextManager {
constructor(public editor: Editor) {} constructor(public editor: Editor) {}
getTextElement() { private textElement: HTMLDivElement | null = null
private getTextElement() {
const oldElm = document.querySelector('.tl-text-measure') const oldElm = document.querySelector('.tl-text-measure')
oldElm?.remove() oldElm?.remove()
@ -64,13 +66,12 @@ export class TextManager {
fontSize: number fontSize: number
lineHeight: number lineHeight: number
/** /**
* When width is a number, the text will be wrapped to that width. When * When maxWidth is a number, the text will be wrapped to that maxWidth. When maxWidth
* width is null, the text will be measured without wrapping, but explicit * is null, the text will be measured without wrapping, but explicit line breaks and
* line breaks and space are preserved. * space are preserved.
*/ */
width: null | number maxWidth: null | number
minWidth?: string minWidth?: string
maxWidth: string
padding: string padding: string
} }
): Box2dModel => { ): Box2dModel => {
@ -82,15 +83,8 @@ export class TextManager {
elm.style.setProperty('font-weight', opts.fontWeight) elm.style.setProperty('font-weight', opts.fontWeight)
elm.style.setProperty('font-size', opts.fontSize + 'px') elm.style.setProperty('font-size', opts.fontSize + 'px')
elm.style.setProperty('line-height', opts.lineHeight * opts.fontSize + 'px') elm.style.setProperty('line-height', opts.lineHeight * opts.fontSize + 'px')
if (opts.width === null) { elm.style.setProperty('max-width', opts.maxWidth === null ? null : opts.maxWidth + 'px')
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('min-width', opts.minWidth ?? null) elm.style.setProperty('min-width', opts.minWidth ?? null)
elm.style.setProperty('max-width', opts.maxWidth)
elm.style.setProperty('padding', opts.padding) elm.style.setProperty('padding', opts.padding)
elm.textContent = normalizeTextForDom(textToMeasure) elm.textContent = normalizeTextForDom(textToMeasure)

View file

@ -289,7 +289,7 @@ export function registerDefaultExternalContentHandlers(
...TEXT_PROPS, ...TEXT_PROPS,
fontFamily: FONT_FAMILIES[defaultProps.font], fontFamily: FONT_FAMILIES[defaultProps.font],
fontSize: FONT_SIZES[defaultProps.size], fontSize: FONT_SIZES[defaultProps.size],
width: null, maxWidth: null,
}) })
const minWidth = Math.min( const minWidth = Math.min(
@ -302,7 +302,7 @@ export function registerDefaultExternalContentHandlers(
...TEXT_PROPS, ...TEXT_PROPS,
fontFamily: FONT_FAMILIES[defaultProps.font], fontFamily: FONT_FAMILIES[defaultProps.font],
fontSize: FONT_SIZES[defaultProps.size], fontSize: FONT_SIZES[defaultProps.size],
width: minWidth, maxWidth: minWidth,
}) })
w = shrunkSize.w w = shrunkSize.w
h = shrunkSize.h h = shrunkSize.h

View file

@ -114,7 +114,7 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
...TEXT_PROPS, ...TEXT_PROPS,
fontFamily: FONT_FAMILIES[shape.props.font], fontFamily: FONT_FAMILIES[shape.props.font],
fontSize: ARROW_LABEL_FONT_SIZES[shape.props.size], fontSize: ARROW_LABEL_FONT_SIZES[shape.props.size],
width: null, maxWidth: null,
}) })
let width = w let width = w
@ -129,7 +129,7 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
...TEXT_PROPS, ...TEXT_PROPS,
fontFamily: FONT_FAMILIES[shape.props.font], fontFamily: FONT_FAMILIES[shape.props.font],
fontSize: ARROW_LABEL_FONT_SIZES[shape.props.size], fontSize: ARROW_LABEL_FONT_SIZES[shape.props.size],
width: width, maxWidth: width,
} }
) )
@ -146,7 +146,7 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
...TEXT_PROPS, ...TEXT_PROPS,
fontFamily: FONT_FAMILIES[shape.props.font], fontFamily: FONT_FAMILIES[shape.props.font],
fontSize: ARROW_LABEL_FONT_SIZES[shape.props.size], fontSize: ARROW_LABEL_FONT_SIZES[shape.props.size],
width: width, maxWidth: width,
} }
) )

View file

@ -1053,8 +1053,7 @@ function getLabelSize(editor: Editor, shape: TLGeoShape) {
...TEXT_PROPS, ...TEXT_PROPS,
fontFamily: FONT_FAMILIES[shape.props.font], fontFamily: FONT_FAMILIES[shape.props.font],
fontSize: LABEL_FONT_SIZES[shape.props.size], fontSize: LABEL_FONT_SIZES[shape.props.size],
width: null, maxWidth: 100,
maxWidth: '100px',
}) })
// TODO: Can I get these from somewhere? // TODO: Can I get these from somewhere?
@ -1069,17 +1068,15 @@ function getLabelSize(editor: Editor, shape: TLGeoShape) {
...TEXT_PROPS, ...TEXT_PROPS,
fontFamily: FONT_FAMILIES[shape.props.font], fontFamily: FONT_FAMILIES[shape.props.font],
fontSize: LABEL_FONT_SIZES[shape.props.size], fontSize: LABEL_FONT_SIZES[shape.props.size],
width: null,
minWidth: minSize.w + 'px', minWidth: minSize.w + 'px',
maxWidth: maxWidth: Math.max(
Math.max(
// Guard because a DOM nodes can't be less 0 // Guard because a DOM nodes can't be less 0
0, 0,
// A 'w' width that we're setting as the min-width // A 'w' width that we're setting as the min-width
Math.ceil(minSize.w + sizes[shape.props.size]), Math.ceil(minSize.w + sizes[shape.props.size]),
// The actual text size // The actual text size
Math.ceil(shape.props.w - LABEL_PADDING * 2) Math.ceil(shape.props.w - LABEL_PADDING * 2)
) + 'px', ),
}) })
return { return {

View file

@ -194,7 +194,7 @@ function getGrowY(editor: Editor, shape: TLNoteShape, prevGrowY = 0) {
...TEXT_PROPS, ...TEXT_PROPS,
fontFamily: FONT_FAMILIES[shape.props.font], fontFamily: FONT_FAMILIES[shape.props.font],
fontSize: LABEL_FONT_SIZES[shape.props.size], fontSize: LABEL_FONT_SIZES[shape.props.size],
width: NOTE_SIZE - PADDING * 2, maxWidth: NOTE_SIZE - PADDING * 2,
}) })
const nextHeight = nextTextSize.h + PADDING * 2 const nextHeight = nextTextSize.h + PADDING * 2

View file

@ -7,7 +7,6 @@ export const TEXT_PROPS = {
fontVariant: 'normal', fontVariant: 'normal',
fontStyle: 'normal', fontStyle: 'normal',
padding: '0px', padding: '0px',
maxWidth: 'auto',
} }
/** @public */ /** @public */

View file

@ -369,7 +369,7 @@ export class TextShapeUtil extends ShapeUtil<TLTextShape> {
function getTextSize(editor: Editor, props: TLTextShape['props']) { function getTextSize(editor: Editor, props: TLTextShape['props']) {
const { font, text, autoSize, size, w } = 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 fontSize = FONT_SIZES[size]
const cw = autoSize const cw = autoSize
@ -381,7 +381,7 @@ function getTextSize(editor: Editor, props: TLTextShape['props']) {
...TEXT_PROPS, ...TEXT_PROPS,
fontFamily: FONT_FAMILIES[font], fontFamily: FONT_FAMILIES[font],
fontSize: fontSize, fontSize: fontSize,
width: cw, maxWidth: cw,
}) })
// // If we're autosizing the measureText will essentially `Math.floor` // // If we're autosizing the measureText will essentially `Math.floor`

View file

@ -81,8 +81,7 @@ export class TestEditor extends Editor {
fontFamily: string fontFamily: string
fontSize: number fontSize: number
lineHeight: number lineHeight: number
width: null | number maxWidth: null | number
maxWidth: string
} }
): Box2dModel => { ): Box2dModel => {
const breaks = textToMeasure.split('\n') const breaks = textToMeasure.split('\n')
@ -95,9 +94,9 @@ export class TestEditor extends Editor {
return { return {
x: 0, x: 0,
y: 0, y: 0,
w: opts.width === null ? w : Math.max(w, opts.width), w: opts.maxWidth === null ? w : Math.max(w, opts.maxWidth),
h: 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, opts.fontSize,
} }
} }
@ -105,9 +104,8 @@ export class TestEditor extends Editor {
this.textMeasure.measureTextSpans = (textToMeasure, opts) => { this.textMeasure.measureTextSpans = (textToMeasure, opts) => {
const box = this.textMeasure.measureText(textToMeasure, { const box = this.textMeasure.measureText(textToMeasure, {
...opts, ...opts,
width: opts.width, maxWidth: opts.width,
padding: `${opts.padding}px`, padding: `${opts.padding}px`,
maxWidth: 'auto',
}) })
return [{ box, text: textToMeasure }] return [{ box, text: textToMeasure }]
} }

View file

@ -87,7 +87,7 @@ exports[`Matches a snapshot: Basic SVG 1`] = `
> >
<g> <g>
<path <path
d="M0, 0L100, 0,100, 100,0, 100Z" d="M0, 0L100, 0,100, 428,0, 428Z"
fill="none" fill="none"
stroke="#1d1d1d" stroke="#1d1d1d"
stroke-width="3.5" stroke-width="3.5"
@ -108,7 +108,7 @@ exports[`Matches a snapshot: Basic SVG 1`] = `
<tspan <tspan
alignment-baseline="mathematical" alignment-baseline="mathematical"
x="16px" x="16px"
y="-181px" y="-17px"
> >
Hello world Hello world
</tspan> </tspan>
@ -127,7 +127,7 @@ exports[`Matches a snapshot: Basic SVG 1`] = `
<tspan <tspan
alignment-baseline="mathematical" alignment-baseline="mathematical"
x="16px" x="16px"
y="-181px" y="-17px"
> >
Hello world Hello world
</tspan> </tspan>