[improvement] bookmark shape logic (#1568)

This PR extracts some logic from the EditUrlDialog into the bookmark
shape util, removing the dependency between the two.

### Change Type

- [x] `internal` — Any other changes that don't affect the published
package (will not publish a new version)

### Test Plan

1. Create a bookmark shape
2. Set its URL to an empty string

- [x] Unit Tests

packages/editor/api-report.md

@@ -216,8 +216,6 @@ export class BookmarkShapeUtil extends BaseBoxShapeUtil<TLBookmarkShape> {
     // (undocumented)
     defaultProps(): TLBookmarkShape['props'];
     // (undocumented)
-    getHumanReadableAddress(shape: TLBookmarkShape): string;
-    // (undocumented)
     hideSelectionBoundsBg: () => boolean;
     // (undocumented)
     hideSelectionBoundsFg: () => boolean;
@@ -231,11 +229,6 @@ export class BookmarkShapeUtil extends BaseBoxShapeUtil<TLBookmarkShape> {
     render(shape: TLBookmarkShape): JSX.Element;
     // (undocumented)
     static type: "bookmark";
-    // (undocumented)
-    protected updateBookmarkAsset: {
-        (shape: TLBookmarkShape): Promise<void>;
-        cancel(): void;
-    };
 }
 
 // @internal (undocumented)
@@ -1257,6 +1250,9 @@ export const isSvgText: (text: string) => boolean;
 // @public (undocumented)
 export const isValidHttpURL: (url: string) => boolean;
 
+// @public (undocumented)
+export function isValidUrl(url: string): boolean;
+
 // @public (undocumented)
 export const LABEL_FONT_SIZES: Record<TLSizeType, number>;
 

packages/editor/src/index.ts

@@ -220,6 +220,7 @@ export {
 	fileToBase64,
 	getIncrementedName,
 	isSerializable,
+	isValidUrl,
 	snapToGrid,
 	uniqueId,
 } from './lib/utils/data'

packages/editor/src/lib/editor/shapes/bookmark/BookmarkShapeUtil.tsx

@@ -2,16 +2,18 @@ import { toDomPrecision } from '@tldraw/primitives'
 import { AssetRecordType, TLAssetId, TLBookmarkAsset, TLBookmarkShape } from '@tldraw/tlschema'
 import { debounce, getHashForString } from '@tldraw/utils'
 import { HTMLContainer } from '../../../components/HTMLContainer'
-import {
-	DEFAULT_BOOKMARK_HEIGHT,
-	DEFAULT_BOOKMARK_WIDTH,
-	ROTATING_SHADOWS,
-} from '../../../constants'
+import { ROTATING_SHADOWS } from '../../../constants'
+
+const DEFAULT_BOOKMARK_WIDTH = 300
+const DEFAULT_BOOKMARK_HEIGHT = 320
+
+import { isValidUrl } from '../../../utils/data'
 import {
 	rotateBoxShadow,
 	stopEventPropagation,
 	truncateStringWithEllipsis,
 } from '../../../utils/dom'
+import { Editor } from '../../Editor'
 import { BaseBoxShapeUtil } from '../BaseBoxShapeUtil'
 import { TLOnBeforeCreateHandler, TLOnBeforeUpdateHandler } from '../ShapeUtil'
 import { HyperlinkButton } from '../shared/HyperlinkButton'
@@ -41,7 +43,7 @@ export class BookmarkShapeUtil extends BaseBoxShapeUtil<TLBookmarkShape> {
 
 		const pageRotation = this.editor.getPageRotation(shape)
 
-		const address = this.getHumanReadableAddress(shape)
+		const address = getHumanReadableAddress(shape)
 
 		return (
 			<HTMLContainer>
@@ -104,68 +106,88 @@ export class BookmarkShapeUtil extends BaseBoxShapeUtil<TLBookmarkShape> {
 	}
 
 	override onBeforeCreate?: TLOnBeforeCreateHandler<TLBookmarkShape> = (shape) => {
-		this.updateBookmarkAsset(shape)
+		updateBookmarkAssetOnUrlChange(this.editor, shape)
 	}
 
 	override onBeforeUpdate?: TLOnBeforeUpdateHandler<TLBookmarkShape> = (prev, shape) => {
 		if (prev.props.url !== shape.props.url) {
-			this.updateBookmarkAsset(shape)
-		}
-	}
-
-	getHumanReadableAddress(shape: TLBookmarkShape) {
-		try {
-			const url = new URL(shape.props.url)
-			const path = url.pathname.replace(/\/*$/, '')
-			return `${url.hostname}${path}`
-		} catch (e) {
-			return shape.props.url
-		}
-	}
-
-	protected updateBookmarkAsset = debounce((shape: TLBookmarkShape) => {
-		const { url } = shape.props
-		const assetId: TLAssetId = AssetRecordType.createId(getHashForString(url))
-		const existing = this.editor.getAssetById(assetId)
-
-		if (existing) {
-			// If there's an existing asset with the same URL, use
-			// its asset id instead.
-			if (shape.props.assetId !== existing.id) {
-				this.editor.updateShapes([
-					{
-						id: shape.id,
-						type: shape.type,
-						props: { assetId },
-					},
-				])
+			if (!isValidUrl(shape.props.url)) {
+				return { ...shape, props: { ...shape.props, url: prev.props.url } }
+			} else {
+				updateBookmarkAssetOnUrlChange(this.editor, shape)
 			}
-		} else {
-			// Create a bookmark asset for the URL. First get its meta
-			// data, then create the asset and update the shape.
-			this.editor.externalContentManager.createAssetFromUrl(this.editor, url).then((asset) => {
-				if (!asset) {
-					this.editor.updateShapes([
-						{
-							id: shape.id,
-							type: shape.type,
-							props: { assetId: undefined },
-						},
-					])
-					return
-				}
-
-				this.editor.batch(() => {
-					this.editor.createAssets([asset])
-					this.editor.updateShapes([
-						{
-							id: shape.id,
-							type: shape.type,
-							props: { assetId: asset.id },
-						},
-					])
-				})
-			})
 		}
-	}, 500)
+	}
 }
+
+/** @internal */
+export const getHumanReadableAddress = (shape: TLBookmarkShape) => {
+	try {
+		const url = new URL(shape.props.url)
+		const path = url.pathname.replace(/\/*$/, '')
+		return `${url.hostname}${path}`
+	} catch (e) {
+		return shape.props.url
+	}
+}
+
+function updateBookmarkAssetOnUrlChange(editor: Editor, shape: TLBookmarkShape) {
+	const { url } = shape.props
+
+	// Derive the asset id from the URL
+	const assetId: TLAssetId = AssetRecordType.createId(getHashForString(url))
+
+	if (editor.getAssetById(assetId)) {
+		// Existing asset for this URL?
+		if (shape.props.assetId !== assetId) {
+			editor.updateShapes([
+				{
+					id: shape.id,
+					type: shape.type,
+					props: { assetId },
+				},
+			])
+		}
+	} else {
+		// No asset for this URL?
+
+		// First, clear out the existing asset reference
+		editor.updateShapes([
+			{
+				id: shape.id,
+				type: shape.type,
+				props: { assetId: null },
+			},
+		])
+
+		// Then try to asyncronously create a new one
+		createBookmarkAssetOnUrlChange(editor, shape)
+	}
+}
+
+const createBookmarkAssetOnUrlChange = debounce(async (editor: Editor, shape: TLBookmarkShape) => {
+	const { url } = shape.props
+
+	// Create the asset using the external content manager's createAssetFromUrl method.
+	// This may be overwritten by the user (for example, we overwrite it on tldraw.com)
+	const asset = await editor.externalContentManager.createAssetFromUrl(editor, url)
+
+	if (!asset) {
+		// No asset? Just leave the bookmark as a null assetId.
+		return
+	}
+
+	editor.batch(() => {
+		// Create the new asset
+		editor.createAssets([asset])
+
+		// And update the shape
+		editor.updateShapes([
+			{
+				id: shape.id,
+				type: shape.type,
+				props: { assetId: asset.id },
+			},
+		])
+	})
+}, 500)

packages/editor/src/lib/editor/shapes/geo/GeoShapeUtil.tsx

@@ -447,7 +447,7 @@ export class GeoShapeUtil extends BaseBoxShapeUtil<TLGeoShape> {
 					labelColor={this.editor.getCssColor(labelColor)}
 					wrap
 				/>
-				{'url' in shape.props && shape.props.url && (
+				{shape.props.url && (
 					<HyperlinkButton url={shape.props.url} zoomLevel={this.editor.zoomLevel} />
 				)}
 			</>

packages/editor/src/lib/editor/shapes/shared/HyperlinkButton.tsx

@@ -8,7 +8,7 @@ export function HyperlinkButton({ url, zoomLevel }: { url: string; zoomLevel: nu
 	return (
 		<a
 			className={classNames('tl-hyperlink-button', {
-				'tl-hyperlink-button__hidden': zoomLevel < 0.5,
+				'tl-hyperlink-button__hidden': zoomLevel < 0.32,
 			})}
 			href={url}
 			target="_blank"
@@ -26,6 +26,5 @@ export function HyperlinkButton({ url, zoomLevel }: { url: string; zoomLevel: nu
 				}}
 			/>
 		</a>
-		// </div>
 	)
 }

packages/editor/src/lib/test/tools/bookmark-shapes.ts

@@ -1,5 +1,8 @@
-import { TLBookmarkShape, createShapeId } from '@tldraw/tlschema'
-import { BookmarkShapeUtil } from '../../editor/shapes/bookmark/BookmarkShapeUtil'
+import { createShapeId, TLBookmarkShape } from '@tldraw/tlschema'
+import {
+	BookmarkShapeUtil,
+	getHumanReadableAddress,
+} from '../../editor/shapes/bookmark/BookmarkShapeUtil'
 import { TestEditor } from '../TestEditor'
 
 let editor: TestEditor
@@ -78,13 +81,12 @@ describe('The URL formatter', () => {
 		const e = editor.getShapeById<TLBookmarkShape>(ids.e)!
 		const f = editor.getShapeById<TLBookmarkShape>(ids.f)!
 
-		const util = editor.getShapeUtil(BookmarkShapeUtil)
-		expect(util.getHumanReadableAddress(a)).toBe('www.github.com')
-		expect(util.getHumanReadableAddress(b)).toBe('www.github.com')
-		expect(util.getHumanReadableAddress(c)).toBe('www.github.com/TodePond')
-		expect(util.getHumanReadableAddress(d)).toBe('www.github.com/TodePond')
-		expect(util.getHumanReadableAddress(e)).toBe('www.github.com')
-		expect(util.getHumanReadableAddress(f)).toBe('www.github.com/TodePond/DreamBerd')
+		expect(getHumanReadableAddress(a)).toBe('www.github.com')
+		expect(getHumanReadableAddress(b)).toBe('www.github.com')
+		expect(getHumanReadableAddress(c)).toBe('www.github.com/TodePond')
+		expect(getHumanReadableAddress(d)).toBe('www.github.com/TodePond')
+		expect(getHumanReadableAddress(e)).toBe('www.github.com')
+		expect(getHumanReadableAddress(f)).toBe('www.github.com/TodePond/DreamBerd')
 	})
 
 	it("Doesn't resize bookmarks", () => {

packages/editor/src/lib/utils/data.ts

@@ -95,3 +95,12 @@ export const checkFlag = (flag: boolean | (() => boolean) | undefined) =>
 export function snapToGrid(n: number, gridSize: number) {
 	return Math.round(n / gridSize) * gridSize
 }
+
+const VALID_URL_REGEX = new RegExp(
+	/^(?:(?:(?:https?|ftp):)?\/\/)(?:\S+(?::\S*)?@)?(?:(?!(?:10|127)(?:\.\d{1,3}){3})(?!(?:169\.254|192\.168)(?:\.\d{1,3}){2})(?!172\.(?:1[6-9]|2\d|3[0-1])(?:\.\d{1,3}){2})(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\u00a1-\uffff0-9]-*)*[a-z\u00a1-\uffff0-9]+)(?:\.(?:[a-z\u00a1-\uffff0-9]-*)*[a-z\u00a1-\uffff0-9]+)*(?:\.(?:[a-z\u00a1-\uffff]{2,})))(?::\d{2,5})?(?:[/?#]\S*)?$/i
+)
+
+/** @public */
+export function isValidUrl(url: string) {
+	return VALID_URL_REGEX.test(url)
+}

packages/ui/src/lib/components/EditLinkDialog.tsx

@@ -1,5 +1,5 @@
-import { BookmarkShapeUtil, TLBaseShape, useEditor } from '@tldraw/editor'
-import { useCallback, useRef, useState } from 'react'
+import { TLBaseShape, isValidUrl, useEditor } from '@tldraw/editor'
+import { useCallback, useEffect, useRef, useState } from 'react'
 import { track } from 'signia-react'
 import { TLUiDialogProps } from '../hooks/useDialogsProvider'
 import { useTranslation } from '../hooks/useTranslation/useTranslation'
@@ -7,16 +7,16 @@ import { Button } from './primitives/Button'
 import * as Dialog from './primitives/Dialog'
 import { Input } from './primitives/Input'
 
-const validUrlRegex = new RegExp(
-	/^(?:(?:(?:https?|ftp):)?\/\/)(?:\S+(?::\S*)?@)?(?:(?!(?:10|127)(?:\.\d{1,3}){3})(?!(?:169\.254|192\.168)(?:\.\d{1,3}){2})(?!172\.(?:1[6-9]|2\d|3[0-1])(?:\.\d{1,3}){2})(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\u00a1-\uffff0-9]-*)*[a-z\u00a1-\uffff0-9]+)(?:\.(?:[a-z\u00a1-\uffff0-9]-*)*[a-z\u00a1-\uffff0-9]+)*(?:\.(?:[a-z\u00a1-\uffff]{2,})))(?::\d{2,5})?(?:[/?#]\S*)?$/i
-)
-
 // A url can either be invalid, or valid with a protocol, or valid without a protocol.
 // For example, "aol.com" would be valid with a protocol ()
 function validateUrl(url: string) {
-	if (validUrlRegex.test(url)) return true
-	if (validUrlRegex.test('https://' + url)) return 'needs protocol'
-	return false
+	if (isValidUrl(url)) {
+		return { isValid: true, hasProtocol: true }
+	}
+	if (isValidUrl('https://' + url)) {
+		return { isValid: true, hasProtocol: false }
+	}
+	return { isValid: false, hasProtocol: false }
 }
 
 type ShapeWithUrl = TLBaseShape<string, { url: string }>
@@ -42,63 +42,82 @@ export const EditLinkDialogInner = track(function EditLinkDialogInner({
 	const editor = useEditor()
 	const msg = useTranslation()
 
-	const [validState, setValid] = useState(validateUrl(selectedShape.props.url))
+	const rInput = useRef<HTMLInputElement>(null)
+
+	useEffect(() => {
+		requestAnimationFrame(() => rInput.current?.focus())
+	}, [])
 
 	const rInitialValue = useRef(selectedShape.props.url)
 
-	const rValue = useRef(selectedShape.props.url)
-	const [urlValue, setUrlValue] = useState<string>(
-		validState
-			? validState === 'needs protocol'
-				? 'https://' + selectedShape.props.url
-				: selectedShape.props.url
-			: 'https://'
-	)
+	const [urlInputState, setUrlInputState] = useState(() => {
+		const urlValidResult = validateUrl(selectedShape.props.url)
+
+		const initialValue =
+			urlValidResult.isValid === true
+				? urlValidResult.hasProtocol
+					? selectedShape.props.url
+					: 'https://' + selectedShape.props.url
+				: 'https://'
+
+		return {
+			actual: initialValue,
+			safe: initialValue,
+			valid: true,
+		}
+	})
 
 	const handleChange = useCallback((rawValue: string) => {
 		// Just auto-correct double https:// from a bad paste.
-		const value = rawValue.replace(/https?:\/\/(https?:\/\/)/, (_match, arg1) => {
+		const fixedRawValue = rawValue.replace(/https?:\/\/(https?:\/\/)/, (_match, arg1) => {
 			return arg1
 		})
-		setUrlValue(value)
 
-		const validStateUrl = validateUrl(value.trim())
-		setValid((s) => (s === validStateUrl ? s : validStateUrl))
-		if (validStateUrl) {
-			rValue.current = value
-		}
+		const urlValidResult = validateUrl(fixedRawValue)
+
+		const safeValue =
+			urlValidResult.isValid === true
+				? urlValidResult.hasProtocol
+					? fixedRawValue
+					: 'https://' + fixedRawValue
+				: 'https://'
+
+		setUrlInputState({
+			actual: fixedRawValue,
+			safe: safeValue,
+			valid: urlValidResult.isValid,
+		})
 	}, [])
 
 	const handleClear = useCallback(() => {
-		editor.setProp('url', '', false)
+		const { onlySelectedShape } = editor
+		if (!onlySelectedShape) return
+		editor.updateShapes([
+			{ id: onlySelectedShape.id, type: onlySelectedShape.type, props: { url: '' } },
+		])
 		onClose()
 	}, [editor, onClose])
 
-	const handleComplete = useCallback(
-		(value: string) => {
-			value = value.trim()
-			const validState = validateUrl(value)
+	const handleComplete = useCallback(() => {
+		const { onlySelectedShape } = editor
 
-			const shape = editor.selectedShapes[0]
+		if (!onlySelectedShape) return
 
-			if (shape && 'url' in shape.props) {
-				const current = shape.props.url
-				const next = validState
-					? validState === 'needs protocol'
-						? 'https://' + value
-						: value
-					: editor.isShapeOfType(shape, BookmarkShapeUtil)
-					? rInitialValue.current
-					: ''
-
-				if (current !== undefined && current !== next) {
-					editor.setProp('url', next, false)
-				}
+		// ? URL is a magic value
+		if (onlySelectedShape && 'url' in onlySelectedShape.props) {
+			// Here would be a good place to validate the next shape—would setting the empty
+			if (onlySelectedShape.props.url !== urlInputState.safe) {
+				editor.updateShapes([
+					{
+						id: onlySelectedShape.id,
+						type: onlySelectedShape.type,
+						props: { url: urlInputState.safe },
+					},
+				])
 			}
-			onClose()
-		},
-		[editor, onClose]
-	)
+		}
+		onClose()
+	}, [editor, onClose, urlInputState])
 
 	const handleCancel = useCallback(() => {
 		onClose()
@@ -111,7 +130,7 @@ export const EditLinkDialogInner = track(function EditLinkDialogInner({
 	}
 
 	// Are we going from a valid state to an invalid state?
-	const isRemoving = rInitialValue.current && !validState
+	const isRemoving = rInitialValue.current && !urlInputState.valid
 
 	return (
 		<>
@@ -122,16 +141,19 @@ export const EditLinkDialogInner = track(function EditLinkDialogInner({
 			<Dialog.Body>
 				<div className="tlui-edit-link-dialog">
 					<Input
+						ref={rInput}
 						className="tlui-edit-link-dialog__input"
 						label="edit-link-dialog.url"
 						autofocus
-						value={urlValue}
+						value={urlInputState.actual}
 						onValueChange={handleChange}
 						onComplete={handleComplete}
 						onCancel={handleCancel}
 					/>
 					<div>
-						{validState ? msg('edit-link-dialog.detail') : msg('edit-link-dialog.invalid-url')}
+						{urlInputState.valid
+							? msg('edit-link-dialog.detail')
+							: msg('edit-link-dialog.invalid-url')}
 					</div>
 				</div>
 			</Dialog.Body>
@@ -146,9 +168,9 @@ export const EditLinkDialogInner = track(function EditLinkDialogInner({
 				) : (
 					<Button
 						type="primary"
-						disabled={!validState}
-						onTouchEnd={() => handleComplete(rValue.current)}
-						onClick={() => handleComplete(rValue.current)}
+						disabled={!urlInputState.valid}
+						onTouchEnd={handleComplete}
+						onClick={handleComplete}
 					>
 						{msg('edit-link-dialog.save')}
 					</Button>

packages/ui/src/lib/hooks/useActions.tsx

@@ -257,8 +257,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
 				readonlyOk: false,
 				onSelect(source) {
 					trackEvent('convert-to-bookmark', { source })
-					const ids = editor.selectedIds
-					const shapes = ids.map((id) => editor.getShapeById(id))
+					const shapes = editor.selectedShapes
 
 					const createList: TLShapePartial[] = []
 					const deleteList: TLShapeId[] = []

packages/ui/src/lib/hooks/useContextMenuSchema.tsx

@@ -77,7 +77,10 @@ export const TLUiContextMenuSchemaProvider = track(function TLUiContextMenuSchem
 			return editor.selectedIds.some((selectedId) => {
 				const shape = editor.getShapeById(selectedId)
 				return (
-					shape && editor.isShapeOfType(shape, BookmarkShapeUtil) && getEmbedInfo(shape.props.url)
+					shape &&
+					editor.isShapeOfType(shape, BookmarkShapeUtil) &&
+					shape.props.url &&
+					getEmbedInfo(shape.props.url)
 				)
 			})
 		},