From 4cc823e22eab5d9e751a32c958e83d28d9eb8ec4 Mon Sep 17 00:00:00 2001 From: Steve Ruiz Date: Fri, 1 Mar 2024 18:16:27 +0000 Subject: [PATCH] Show a broken image for files without assets (#2990) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR shows a broken state for images or video shapes without assets. It deletes assets when pasted content fails to generate a new asset for the shape. It includes some mild refactoring to the image shape. Previously, shapes that had no corresponding assets would be transparent. This PR preserves the transparent state for shapes with assets but without source data (ie loading assets). After: image image Before: image ### Change Type - [x] `patch` — Bug fix ### Test Plan 1. Create an image / video 2. Delete its asset ### Release Notes - Better handling of broken images / videos. --- lerna.json | 4 +- packages/editor/editor.css | 1 + packages/editor/src/lib/editor/Editor.ts | 64 ++-- packages/namespaced-tldraw/tldraw.css | 1 + packages/tldraw/api-report.md | 2 - packages/tldraw/api/api.json | 48 --- .../src/lib/shapes/image/ImageShapeUtil.tsx | 152 ++++----- .../src/lib/shapes/shared/BrokenAssetIcon.tsx | 18 ++ .../src/lib/shapes/video/VideoShapeUtil.tsx | 305 +++++++++--------- 9 files changed, 285 insertions(+), 310 deletions(-) create mode 100644 packages/tldraw/src/lib/shapes/shared/BrokenAssetIcon.tsx diff --git a/lerna.json b/lerna.json index 45455ce09..5ea4fee74 100644 --- a/lerna.json +++ b/lerna.json @@ -1,7 +1,5 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "packages": [ - "packages/*" - ], + "packages": ["packages/*"], "version": "2.0.0" } diff --git a/packages/editor/editor.css b/packages/editor/editor.css index e10e7f18d..78730becc 100644 --- a/packages/editor/editor.css +++ b/packages/editor/editor.css @@ -546,6 +546,7 @@ input, height: 100%; } +.tl-video-container, .tl-image-container, .tl-embed-container { width: 100%; diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index 6bebbc141..4e4e2c691 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -7768,6 +7768,7 @@ export class Editor extends EventEmitter { } } + // Ok, we've got our migrated shapes and assets, now we can continue! const idMap = new Map(shapes.map((shape) => [shape.id, createShapeId()])) // By default, the paste parent will be the current page. @@ -7905,54 +7906,57 @@ export class Editor extends EventEmitter { return this } - // Migrate the new shapes - - let assetsToCreate: TLAsset[] = [] + // These are all the assets we need to create + const assetsToCreate: TLAsset[] = [] + // These assets have base64 data that may need to be hosted const assetsToUpdate: (TLImageAsset | TLVideoAsset)[] = [] - assetsToCreate = assets - .filter((asset) => !this.store.has(asset.id)) - .map((asset) => { - if (asset.type === 'image' || asset.type === 'video') { - if (asset.props.src && asset.props.src?.startsWith('data:image')) { - assetsToUpdate.push(structuredClone(asset)) - asset.props.src = null - } else { - assetsToUpdate.push(structuredClone(asset)) - } - } + for (const asset of assets) { + if (this.store.has(asset.id)) { + // We already have this asset + continue + } - return asset - }) + if ( + (asset.type === 'image' || asset.type === 'video') && + asset.props.src?.startsWith('data:image') + ) { + // it's src is a base64 image or video; we need to create a new asset without the src, + // then create a new asset from the original src. So we save a copy of the original asset, + // then delete the src from the original asset. + assetsToUpdate.push(structuredClone(asset as TLImageAsset | TLVideoAsset)) + asset.props.src = null + } + // Add the asset to the list of assets to create + assetsToCreate.push(asset) + } + + // Start loading the new assets, order does not matter Promise.allSettled( - assetsToUpdate.map(async (asset) => { + (assetsToUpdate as (TLImageAsset | TLVideoAsset)[]).map(async (asset) => { + // Turn the data url into a file const file = await dataUrlToFile( asset.props.src!, asset.props.name, asset.props.mimeType ?? 'image/png' ) + // Get a new asset for the file const newAsset = await this.getAssetForExternalContent({ type: 'file', file }) if (!newAsset) { - return null + // If we don't have a new asset, delete the old asset. + // The shapes that reference this asset should break. + this.deleteAssets([asset.id]) + return } - return [asset, newAsset] as const + // Save the new asset under the old asset's id + this.updateAssets([{ ...newAsset, id: asset.id }]) }) - ).then((assets) => { - this.updateAssets( - compact( - assets.map((result) => - result.status === 'fulfilled' && result.value - ? { ...result.value[1], id: result.value[0].id } - : undefined - ) - ) - ) - }) + ) this.batch(() => { // Create any assets that need to be created diff --git a/packages/namespaced-tldraw/tldraw.css b/packages/namespaced-tldraw/tldraw.css index 107ed6606..ef9a2899b 100644 --- a/packages/namespaced-tldraw/tldraw.css +++ b/packages/namespaced-tldraw/tldraw.css @@ -550,6 +550,7 @@ input, height: 100%; } +.tl-video-container, .tl-image-container, .tl-embed-container { width: 100%; diff --git a/packages/tldraw/api-report.md b/packages/tldraw/api-report.md index d217a8b17..3137a139c 100644 --- a/packages/tldraw/api-report.md +++ b/packages/tldraw/api-report.md @@ -896,8 +896,6 @@ export class ImageShapeUtil extends BaseBoxShapeUtil { } | null>; }; // (undocumented) - shouldGetDataURI(src: string): "" | boolean; - // (undocumented) toSvg(shape: TLImageShape): Promise; // (undocumented) static type: "image"; diff --git a/packages/tldraw/api/api.json b/packages/tldraw/api/api.json index 254b9b788..848018a88 100644 --- a/packages/tldraw/api/api.json +++ b/packages/tldraw/api/api.json @@ -10424,54 +10424,6 @@ "isProtected": false, "isAbstract": false }, - { - "kind": "Method", - "canonicalReference": "tldraw!ImageShapeUtil#shouldGetDataURI:member(1)", - "docComment": "", - "excerptTokens": [ - { - "kind": "Content", - "text": "shouldGetDataURI(src: " - }, - { - "kind": "Content", - "text": "string" - }, - { - "kind": "Content", - "text": "): " - }, - { - "kind": "Content", - "text": "\"\" | boolean" - }, - { - "kind": "Content", - "text": ";" - } - ], - "isStatic": false, - "returnTypeTokenRange": { - "startIndex": 3, - "endIndex": 4 - }, - "releaseTag": "Public", - "isProtected": false, - "overloadIndex": 1, - "parameters": [ - { - "parameterName": "src", - "parameterTypeTokenRange": { - "startIndex": 1, - "endIndex": 2 - }, - "isOptional": false - } - ], - "isOptional": false, - "isAbstract": false, - "name": "shouldGetDataURI" - }, { "kind": "Method", "canonicalReference": "tldraw!ImageShapeUtil#toSvg:member(1)", diff --git a/packages/tldraw/src/lib/shapes/image/ImageShapeUtil.tsx b/packages/tldraw/src/lib/shapes/image/ImageShapeUtil.tsx index 7f2656952..784a4db90 100644 --- a/packages/tldraw/src/lib/shapes/image/ImageShapeUtil.tsx +++ b/packages/tldraw/src/lib/shapes/image/ImageShapeUtil.tsx @@ -10,37 +10,12 @@ import { imageShapeMigrations, imageShapeProps, toDomPrecision, - useIsCropping, - useValue, } from '@tldraw/editor' import { useEffect, useState } from 'react' +import { BrokenAssetIcon } from '../shared/BrokenAssetIcon' import { HyperlinkButton } from '../shared/HyperlinkButton' import { usePrefersReducedMotion } from '../shared/usePrefersReducedMotion' -const loadImage = async (url: string): Promise => { - return new Promise((resolve, reject) => { - const image = new Image() - image.onload = () => resolve(image) - image.onerror = () => reject(new Error('Failed to load image')) - image.crossOrigin = 'anonymous' - image.src = url - }) -} - -const getStateFrame = async (url: string) => { - const image = await loadImage(url) - - const canvas = document.createElement('canvas') - canvas.width = image.width - canvas.height = image.height - - const ctx = canvas.getContext('2d') - if (!ctx) return - - ctx.drawImage(image, 0, 0) - return canvas.toDataURL() -} - async function getDataURIFromURL(url: string): Promise { const response = await fetch(url) const blob = await response.blob() @@ -73,23 +48,47 @@ export class ImageShapeUtil extends BaseBoxShapeUtil { } component(shape: TLImageShape) { - const containerStyle = getContainerStyle(shape) - const isCropping = useIsCropping(shape.id) + const isCropping = this.editor.getCroppingShapeId() === shape.id const prefersReducedMotion = usePrefersReducedMotion() const [staticFrameSrc, setStaticFrameSrc] = useState('') const asset = shape.props.assetId ? this.editor.getAsset(shape.props.assetId) : undefined + const isSelected = shape.id === this.editor.getOnlySelectedShape()?.id + + useEffect(() => { + if (asset?.props.src && 'mimeType' in asset.props && asset?.props.mimeType === 'image/gif') { + let cancelled = false + const url = asset.props.src + if (!url) return + + const image = new Image() + image.onload = () => { + if (cancelled) return + + const canvas = document.createElement('canvas') + canvas.width = image.width + canvas.height = image.height + + const ctx = canvas.getContext('2d') + if (!ctx) return + + ctx.drawImage(image, 0, 0) + setStaticFrameSrc(canvas.toDataURL()) + } + image.crossOrigin = 'anonymous' + image.src = url + + return () => { + cancelled = true + } + } + }, [prefersReducedMotion, asset?.props]) + if (asset?.type === 'bookmark') { throw Error("Bookmark assets can't be rendered as images") } - const isSelected = useValue( - 'onlySelectedShape', - () => shape.id === this.editor.getOnlySelectedShape()?.id, - [this.editor] - ) - const showCropPreview = isSelected && isCropping && @@ -100,27 +99,35 @@ export class ImageShapeUtil extends BaseBoxShapeUtil { prefersReducedMotion && (asset?.props.mimeType?.includes('video') || asset?.props.mimeType?.includes('gif')) - useEffect(() => { - if (asset?.props.src && 'mimeType' in asset.props && asset?.props.mimeType === 'image/gif') { - let cancelled = false - const run = async () => { - const newStaticFrame = await getStateFrame(asset.props.src!) - if (cancelled) return - if (newStaticFrame) { - setStaticFrameSrc(newStaticFrame) - } - } - run() + const containerStyle = getCroppedContainerStyle(shape) - return () => { - cancelled = true - } - } - }, [prefersReducedMotion, asset?.props]) + if (!asset?.props.src) { + return ( + +
+ {asset ? null : } +
+ ) + {'url' in shape.props && shape.props.url && ( + + )} + + ) + } return ( <> - {asset?.props.src && showCropPreview && ( + {showCropPreview && (
{ style={{ overflow: 'hidden', width: shape.props.w, height: shape.props.h }} >
- {asset?.props.src ? ( -
- ) : null} - {asset?.props.isAnimated && !shape.props.playing && ( +
+ {asset.props.isAnimated && !shape.props.playing && (
GIF
)}
- {'url' in shape.props && shape.props.url && ( + ) + {shape.props.url && ( )} @@ -163,30 +169,26 @@ export class ImageShapeUtil extends BaseBoxShapeUtil { } indicator(shape: TLImageShape) { - const isCropping = useIsCropping(shape.id) - if (isCropping) { - return null - } + const isCropping = this.editor.getCroppingShapeId() === shape.id + if (isCropping) return null return } - shouldGetDataURI(src: string) { - return src && (src.startsWith('http') || src.startsWith('/') || src.startsWith('./')) - } - override async toSvg(shape: TLImageShape) { const g = document.createElementNS('http://www.w3.org/2000/svg', 'g') const asset = shape.props.assetId ? this.editor.getAsset(shape.props.assetId) : null + if (!asset) return g + let src = asset?.props.src || '' - if (this.shouldGetDataURI(src)) { + if (src.startsWith('http') || src.startsWith('/') || src.startsWith('./')) { // If it's a remote image, we need to fetch it and convert it to a data URI src = (await getDataURIFromURL(src)) || '' } const image = document.createElementNS('http://www.w3.org/2000/svg', 'image') image.setAttributeNS('http://www.w3.org/1999/xlink', 'href', src) - const containerStyle = getContainerStyle(shape) + const containerStyle = getCroppedContainerStyle(shape) const crop = shape.props.crop if (containerStyle.transform && crop) { const { transform, width, height } = containerStyle @@ -294,7 +296,7 @@ export class ImageShapeUtil extends BaseBoxShapeUtil { * @param shape - Shape The image shape for which to get the container style * @returns - Styles to apply to the image container */ -function getContainerStyle(shape: TLImageShape) { +function getCroppedContainerStyle(shape: TLImageShape) { const crop = shape.props.crop const topLeft = crop?.topLeft if (!topLeft) { diff --git a/packages/tldraw/src/lib/shapes/shared/BrokenAssetIcon.tsx b/packages/tldraw/src/lib/shapes/shared/BrokenAssetIcon.tsx new file mode 100644 index 000000000..8b6197bd4 --- /dev/null +++ b/packages/tldraw/src/lib/shapes/shared/BrokenAssetIcon.tsx @@ -0,0 +1,18 @@ +export function BrokenAssetIcon() { + return ( + + + + + + ) +} diff --git a/packages/tldraw/src/lib/shapes/video/VideoShapeUtil.tsx b/packages/tldraw/src/lib/shapes/video/VideoShapeUtil.tsx index 8c0a5b43f..8c0ffb493 100644 --- a/packages/tldraw/src/lib/shapes/video/VideoShapeUtil.tsx +++ b/packages/tldraw/src/lib/shapes/video/VideoShapeUtil.tsx @@ -1,14 +1,15 @@ +/* eslint-disable react-hooks/rules-of-hooks */ import { BaseBoxShapeUtil, HTMLContainer, TLVideoShape, toDomPrecision, - track, useIsEditing, videoShapeMigrations, videoShapeProps, } from '@tldraw/editor' -import React from 'react' +import { ReactEventHandler, useCallback, useEffect, useRef, useState } from 'react' +import { BrokenAssetIcon } from '../shared/BrokenAssetIcon' import { HyperlinkButton } from '../shared/HyperlinkButton' import { usePrefersReducedMotion } from '../shared/usePrefersReducedMotion' @@ -33,7 +34,156 @@ export class VideoShapeUtil extends BaseBoxShapeUtil { } component(shape: TLVideoShape) { - return + const { editor } = this + const showControls = editor.getShapeGeometry(shape).bounds.w * editor.getZoomLevel() >= 110 + const asset = shape.props.assetId ? editor.getAsset(shape.props.assetId) : null + const { time, playing } = shape.props + const isEditing = useIsEditing(shape.id) + const prefersReducedMotion = usePrefersReducedMotion() + + const rVideo = useRef(null!) + + const handlePlay = useCallback>( + (e) => { + const video = e.currentTarget + + editor.updateShapes([ + { + type: 'video', + id: shape.id, + props: { + playing: true, + time: video.currentTime, + }, + }, + ]) + }, + [shape.id, editor] + ) + + const handlePause = useCallback>( + (e) => { + const video = e.currentTarget + + editor.updateShapes([ + { + type: 'video', + id: shape.id, + props: { + playing: false, + time: video.currentTime, + }, + }, + ]) + }, + [shape.id, editor] + ) + + const handleSetCurrentTime = useCallback>( + (e) => { + const video = e.currentTarget + + if (isEditing) { + editor.updateShapes([ + { + type: 'video', + id: shape.id, + props: { + time: video.currentTime, + }, + }, + ]) + } + }, + [isEditing, shape.id, editor] + ) + + const [isLoaded, setIsLoaded] = useState(false) + + const handleLoadedData = useCallback>( + (e) => { + const video = e.currentTarget + if (time !== video.currentTime) { + video.currentTime = time + } + + if (!playing) { + video.pause() + } + + setIsLoaded(true) + }, + [playing, time] + ) + + // If the current time changes and we're not editing the video, update the video time + useEffect(() => { + const video = rVideo.current + + if (!video) return + + if (isLoaded && !isEditing && time !== video.currentTime) { + video.currentTime = time + } + + if (isEditing) { + if (document.activeElement !== video) { + video.focus() + } + } + }, [isEditing, isLoaded, time]) + + useEffect(() => { + if (prefersReducedMotion) { + const video = rVideo.current + video.pause() + video.currentTime = 0 + } + }, [rVideo, prefersReducedMotion]) + + return ( + <> + + {asset?.props.src ? ( + + ) : ( + + )} + + {'url' in shape.props && shape.props.url && ( + + )} + + ) } indicator(shape: TLVideoShape) { @@ -64,152 +214,3 @@ function serializeVideo(id: string): string { return canvas.toDataURL('image/png') } else throw new Error('Video with not found when attempting serialization.') } - -const TLVideoUtilComponent = track(function TLVideoUtilComponent(props: { - shape: TLVideoShape - videoUtil: VideoShapeUtil -}) { - const { shape, videoUtil } = props - const showControls = - videoUtil.editor.getShapeGeometry(shape).bounds.w * videoUtil.editor.getZoomLevel() >= 110 - const asset = shape.props.assetId ? videoUtil.editor.getAsset(shape.props.assetId) : null - const { time, playing } = shape.props - const isEditing = useIsEditing(shape.id) - const prefersReducedMotion = usePrefersReducedMotion() - - const rVideo = React.useRef(null!) - - const handlePlay = React.useCallback>( - (e) => { - const video = e.currentTarget - - videoUtil.editor.updateShapes([ - { - type: 'video', - id: shape.id, - props: { - playing: true, - time: video.currentTime, - }, - }, - ]) - }, - [shape.id, videoUtil.editor] - ) - - const handlePause = React.useCallback>( - (e) => { - const video = e.currentTarget - - videoUtil.editor.updateShapes([ - { - type: 'video', - id: shape.id, - props: { - playing: false, - time: video.currentTime, - }, - }, - ]) - }, - [shape.id, videoUtil.editor] - ) - - const handleSetCurrentTime = React.useCallback>( - (e) => { - const video = e.currentTarget - - if (isEditing) { - videoUtil.editor.updateShapes([ - { - type: 'video', - id: shape.id, - props: { - time: video.currentTime, - }, - }, - ]) - } - }, - [isEditing, shape.id, videoUtil.editor] - ) - - const [isLoaded, setIsLoaded] = React.useState(false) - - const handleLoadedData = React.useCallback>( - (e) => { - const video = e.currentTarget - if (time !== video.currentTime) { - video.currentTime = time - } - - if (!playing) { - video.pause() - } - - setIsLoaded(true) - }, - [playing, time] - ) - - // If the current time changes and we're not editing the video, update the video time - React.useEffect(() => { - const video = rVideo.current - - if (!video) return - - if (isLoaded && !isEditing && time !== video.currentTime) { - video.currentTime = time - } - - if (isEditing) { - if (document.activeElement !== video) { - video.focus() - } - } - }, [isEditing, isLoaded, time]) - - React.useEffect(() => { - if (prefersReducedMotion) { - const video = rVideo.current - video.pause() - video.currentTime = 0 - } - }, [rVideo, prefersReducedMotion]) - - return ( - <> - -
- {asset?.props.src ? ( - - ) : null} -
- - {'url' in shape.props && shape.props.url && ( - - )} - - ) -})