Use Suspense for Image loading to reduce the cognitive overload in th…

…e code and also utilize the react18 concurrent rendering Signed-off-by: MTRNord <[email protected]> Fix formatting Signed-off-by: MTRNord <[email protected]>
nordeck · Nov 13, 2024 · 00f0450 · 00f0450
1 parent b7d3906
commit 00f0450
Show file tree

Hide file tree

Showing 4 changed files with 181 additions and 144 deletions.
diff --git a/packages/react-sdk/package.json b/packages/react-sdk/package.json
@@ -36,13 +36,15 @@
     "react-beautiful-dnd": "^13.1.1",
     "react-draggable": "^4.4.6",
     "react-dropzone": "^14.2.9",
+    "react-error-boundary": "^4.1.2",
     "react-hotkeys-hook": "^4.5.0",
     "react-i18next": "^15.0.1",
     "react-joyride": "^2.9.2",
     "react-redux": "^9.1.2",
     "react-transition-group": "^4.4.5",
     "react-use": "^17.5.1",
     "rxjs": "^7.8.1",
+    "swr": "^2.2.5",
     "tinycolor2": "^1.6.0",
     "yjs": "^13.6.19"
   },
@@ -73,13 +75,13 @@
     "i18next-parser": "^9.0.2",
     "i18next-resources-to-backend": "^1.2.1",
     "jsdom": "^25.0.1",
+    "pdfjs-dist": "^4.6.82",
     "prettier": "^3.3.3",
     "typescript": "^5.6.3",
     "vite": "^5.4.10",
     "vitest": "^2.1.4",
     "vitest-fetch-mock": "^0.4.1",
-    "yarn-deduplicate": "^6.0.2",
-    "pdfjs-dist": "^4.6.82"
+    "yarn-deduplicate": "^6.0.2"
   },
   "scripts": {
     "build": "tsc",

diff --git a/packages/react-sdk/src/components/Whiteboard/Element/ConnectedElement.tsx b/packages/react-sdk/src/components/Whiteboard/Element/ConnectedElement.tsx
@@ -15,10 +15,14 @@
  */
 
 import { useWidgetApi } from '@matrix-widget-toolkit/react';
+import { Suspense } from 'react';
+import { ErrorBoundary } from 'react-error-boundary';
 import { Elements } from '../../../state/types';
 import { useElementOverride } from '../../ElementOverridesProvider';
 import EllipseDisplay from '../../elements/ellipse/Display';
 import ImageDisplay from '../../elements/image/ImageDisplay';
+import { ImagePlaceholder } from '../../elements/image/ImagePlaceholder';
+import { Skeleton } from '../../elements/image/Skeleton';
 import LineDisplay from '../../elements/line/Display';
 import PolylineDisplay from '../../elements/polyline/Display';
 import RectangleDisplay from '../../elements/rectangle/Display';
@@ -72,11 +76,34 @@ export const ConnectedElement = ({
       }
 
       return (
-        <ImageDisplay
-          baseUrl={widgetApi.widgetParameters.baseUrl}
-          {...element}
-          {...otherProps}
-        />
+        <Suspense
+          fallback={
+            <Skeleton
+              data-testid={`element-${otherProps.elementId}-skeleton`}
+              x={element.position.x}
+              y={element.position.y}
+              width={element.width}
+              height={element.height}
+            />
+          }
+        >
+          <ErrorBoundary
+            fallback={
+              <ImagePlaceholder
+                position={element.position}
+                width={element.width}
+                height={element.height}
+                elementId={otherProps.elementId}
+              />
+            }
+          >
+            <ImageDisplay
+              baseUrl={widgetApi.widgetParameters.baseUrl}
+              {...element}
+              {...otherProps}
+            />
+          </ErrorBoundary>
+        </Suspense>
       );
     }
   }

diff --git a/packages/react-sdk/src/components/elements/image/ImageDisplay.tsx b/packages/react-sdk/src/components/elements/image/ImageDisplay.tsx
@@ -14,10 +14,12 @@
  * limitations under the License.
  */
 
+import { WidgetApi } from '@matrix-widget-toolkit/api';
 import { useWidgetApi } from '@matrix-widget-toolkit/react';
 import { styled } from '@mui/material';
 import { IDownloadFileActionFromWidgetResponseData } from 'matrix-widget-api';
-import React, { useCallback, useEffect, useState } from 'react';
+import React, { useCallback } from 'react';
+import useSWR from 'swr';
 import { convertMxcToHttpUrl, WidgetApiActionError } from '../../../lib';
 import { ImageElement } from '../../../state';
 import {
@@ -26,8 +28,100 @@ import {
   SelectableElement,
   WithExtendedSelectionProps,
 } from '../../Whiteboard';
-import { ImagePlaceholder } from './ImagePlaceholder';
-import { Skeleton } from './Skeleton';
+
+const downloadFile = async ({
+  widgetApi,
+  baseUrl,
+  mxc,
+  mimeType,
+}: {
+  widgetApi: WidgetApi;
+  baseUrl: string;
+  mxc: string;
+  mimeType: string;
+}) => {
+  try {
+    const result = await tryDownloadFileWithWidgetApi(widgetApi, mxc);
+    const blob = getBlobFromResult(result, mimeType);
+    const downloadedFileDataUrl = createObjectUrlFromBlob(blob);
+    return downloadedFileDataUrl;
+  } catch (error) {
+    handleDownloadError(error as Error, mxc, baseUrl, mimeType);
+  }
+};
+
+const tryDownloadFileWithWidgetApi = async (
+  widgetApi: WidgetApi,
+  mxc: string,
+) => {
+  try {
+    const result = await widgetApi.downloadFile(mxc);
+    return result;
+  } catch {
+    throw new WidgetApiActionError('downloadFile not available');
+  }
+};
+
+const getBlobFromResult = (
+  result: IDownloadFileActionFromWidgetResponseData,
+  mimeType: string,
+): Blob => {
+  if (!(result.file instanceof Blob)) {
+    throw new Error('Got non Blob file response');
+  }
+  return result.file.slice(0, result.file.size, mimeType);
+};
+
+const createObjectUrlFromBlob = (blob: Blob): string => {
+  const url = URL.createObjectURL(blob);
+  if (url === '') {
+    throw new Error('Failed to create object URL');
+  }
+  return url;
+};
+
+const handleDownloadError = (
+  error: Error,
+  mxc: string,
+  baseUrl: string,
+  mimeType: string,
+) => {
+  if (error instanceof WidgetApiActionError) {
+    tryFallbackDownload(mxc, baseUrl, mimeType);
+  } else {
+    throw error;
+  }
+};
+
+const tryFallbackDownload = async (
+  mxc: string,
+  baseUrl: string,
+  mimeType: string,
+) => {
+  let abortController: AbortController | undefined;
+
+  const httpUrl = convertMxcToHttpUrl(mxc, baseUrl);
+
+  if (httpUrl === null) {
+    return '';
+  }
+
+  if (mimeType === 'image/svg+xml') {
+    abortController = new AbortController();
+    try {
+      const response = await fetch(httpUrl, {
+        signal: abortController.signal,
+      });
+      const rawBlob = await response.blob();
+      const svgBlob = rawBlob.slice(0, rawBlob.size, mimeType);
+      return URL.createObjectURL(svgBlob);
+    } catch (fetchError) {
+      console.error('Failed to fetch SVG image:', fetchError);
+      throw new Error('Failed to fetch SVG image');
+    }
+  }
+  return httpUrl;
+};
 
 type ImageDisplayProps = Omit<ImageElement, 'kind'> &
   WithExtendedSelectionProps & {
@@ -37,13 +131,12 @@ type ImageDisplayProps = Omit<ImageElement, 'kind'> &
     baseUrl: string;
   };
 
-const Image = styled('image', {
-  shouldForwardProp: (p) => p !== 'loading',
-})<{ loading: boolean }>(({ loading }) => ({
+const Image = styled(
+  'image',
+  {},
+)<{}>(() => ({
   userSelect: 'none',
   WebkitUserSelect: 'none',
-  // prevention of partial rendering if the image has not yet been fully loaded
-  visibility: loading ? 'hidden' : 'visible',
 }));
 
 /**
@@ -64,132 +157,21 @@ function ImageDisplay({
   overrides = {},
 }: ImageDisplayProps) {
   const widgetApi = useWidgetApi();
-  const [loadError, setLoadError] = useState(false);
-  const [loading, setLoading] = useState(true);
-  const [imageUri, setImageUri] = useState<undefined | string>();
+  const { data: imageUri } = useSWR(
+    { widgetApi, baseUrl, mxc, mimeType },
+    downloadFile,
+    { suspense: true },
+  );
 
   const handleLoad = useCallback(() => {
-    setLoading(false);
-    setLoadError(false);
-
     // This can happen directly when the image is loaded and saves some memory.
     if (imageUri) {
       URL.revokeObjectURL(imageUri);
     }
-  }, [setLoading, setLoadError, imageUri]);
-
-  const handleLoadError = useCallback(() => {
-    setLoading(false);
-    setLoadError(true);
-  }, [setLoading, setLoadError]);
-
-  useEffect(() => {
-    const downloadFile = async () => {
-      try {
-        const result = await tryDownloadFileWithWidgetApi(mxc);
-        const blob = getBlobFromResult(result, mimeType);
-        const downloadedFileDataUrl = createObjectUrlFromBlob(blob);
-        setImageUri(downloadedFileDataUrl);
-      } catch (error) {
-        handleDownloadError(error as Error);
-      }
-    };
-
-    const tryDownloadFileWithWidgetApi = async (mxc: string) => {
-      try {
-        const result = await widgetApi.downloadFile(mxc);
-        return result;
-      } catch {
-        throw new WidgetApiActionError('downloadFile not available');
-      }
-    };
-
-    const getBlobFromResult = (
-      result: IDownloadFileActionFromWidgetResponseData,
-      mimeType: string,
-    ): Blob => {
-      if (!(result.file instanceof Blob)) {
-        throw new Error('Got non Blob file response');
-      }
-      return result.file.slice(0, result.file.size, mimeType);
-    };
-
-    const createObjectUrlFromBlob = (blob: Blob): string => {
-      const url = URL.createObjectURL(blob);
-      if (url === '') {
-        throw new Error('Failed to create object URL');
-      }
-      return url;
-    };
-
-    const handleDownloadError = (error: Error) => {
-      if (error instanceof WidgetApiActionError) {
-        tryFallbackDownload();
-      } else {
-        setLoadError(true);
-      }
-    };
-
-    const tryFallbackDownload = async () => {
-      let abortController: AbortController | undefined;
-
-      const httpUrl = convertMxcToHttpUrl(mxc, baseUrl);
-
-      if (httpUrl === null) {
-        setImageUri('');
-        return;
-      }
-
-      if (mimeType === 'image/svg+xml') {
-        abortController = new AbortController();
-        try {
-          const response = await fetch(httpUrl, {
-            signal: abortController.signal,
-          });
-          const rawBlob = await response.blob();
-          const svgBlob = rawBlob.slice(0, rawBlob.size, mimeType);
-          setImageUri(URL.createObjectURL(svgBlob));
-        } catch (fetchError) {
-          console.error('Failed to fetch SVG image:', fetchError);
-          setLoadError(true);
-        }
-        return;
-      }
-
-      setImageUri(httpUrl);
-
-      return () => {
-        if (abortController) {
-          abortController.abort();
-        }
-      };
-    };
-
-    downloadFile();
-  }, [baseUrl, mimeType, mxc, widgetApi]);
-
-  const renderedSkeleton =
-    loading && !loadError ? (
-      <Skeleton
-        data-testid={`element-${elementId}-skeleton`}
-        x={position.x}
-        y={position.y}
-        width={width}
-        height={height}
-      />
-    ) : null;
-
-  const renderedPlaceholder = loadError ? (
-    <ImagePlaceholder
-      position={position}
-      width={width}
-      height={height}
-      elementId={elementId}
-    />
-  ) : null;
+  }, [imageUri]);
 
   const renderedChild =
-    imageUri !== undefined && !loadError ? (
+    imageUri !== undefined ? (
       <Image
         data-testid={`element-${elementId}-image`}
         href={imageUri}
@@ -199,17 +181,11 @@ function ImageDisplay({
         height={height}
         preserveAspectRatio="none"
         onLoad={handleLoad}
-        onError={handleLoadError}
-        loading={loading}
       />
     ) : null;
 
   if (readOnly) {
-    return (
-      <>
-        {renderedSkeleton} {renderedChild} {renderedPlaceholder}
-      </>
-    );
+    return <>{renderedChild}</>;
   }
 
   return (
@@ -221,9 +197,7 @@ function ImageDisplay({
       >
         <MoveableElement elementId={elementId} overrides={overrides}>
           <ElementContextMenu activeElementIds={activeElementIds}>
-            {renderedSkeleton}
             {renderedChild}
-            {renderedPlaceholder}
           </ElementContextMenu>
         </MoveableElement>
       </SelectableElement>