fix(url-preview): stop cards holding placeholder height and resolve previews ahead of the viewport

.changeset/fix-link-preview-whitespace.md

@@ -0,0 +1,5 @@
+---
+default: patch
+---
+
+# Fix link previews leaving empty space in a message until it is reloaded

src/app/components/url-preview/UrlPreviewCard.tsx

@@ -1,8 +1,7 @@
-import type { MutableRefObject } from 'react';
-import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react';
+import { Children, useCallback, useEffect, useRef, useState } from 'react';
 import type { MatrixClient } from '$types/matrix-sdk';
 import type { IPreviewUrlResponse } from '$types/matrix-sdk';
-import { Box, IconButton, Scroll, Spinner, Text, as, color, config, toRem } from 'folds';
+import { Box, IconButton, Scroll, Text, as, color, config, toRem } from 'folds';
 import { ArrowLeft, ArrowRight, sizedIcon } from '$components/icons/phosphor';
 import { AsyncStatus, useAsyncCallback } from '$hooks/useAsyncCallback';
 import { useMatrixClient } from '$hooks/useMatrixClient';
@@ -62,6 +61,39 @@ const rememberPreview = (mx: MatrixClient, url: string, settled: SettledPreview)
   if (resultCache.size > PREVIEW_RESULT_LIMIT && !oldest.done) resultCache.delete(oldest.value);
 };
 
+const requestUrlPreview = (
+  mx: MatrixClient,
+  url: string,
+  ts: number
+): Promise<IPreviewUrlResponse | null> => {
+  const remembered = getResultCache(mx).get(url);
+  if (remembered) {
+    return 'failed' in remembered
+      ? Promise.reject(new Error('preview previously refused'))
+      : Promise.resolve(remembered.data);
+  }
+  const clientCache = getClientCache(mx);
+  const cached = clientCache.get(url);
+  if (cached !== undefined) return cached;
+  const previewResult = mx?.getUrlPreview(url, ts);
+  if (!previewResult) return Promise.resolve(null);
+  clientCache.set(url, previewResult);
+  previewResult
+    .then((data) => rememberPreview(mx, url, { data }))
+    .catch(() => rememberPreview(mx, url, { failed: true }))
+    .finally(() => clientCache.delete(url));
+  return previewResult;
+};
+
+// A card whose result is already cached renders at its final height, so it never grows in place.
+export const prefetchUrlPreview = (mx: MatrixClient, url: string, ts: number): Promise<void> => {
+  if (getResultCache(mx).has(url) || getClientCache(mx).has(url)) return Promise.resolve();
+  return requestUrlPreview(mx, url, ts).then(
+    () => undefined,
+    () => undefined
+  );
+};
+
 const openMediaInNewTab = async (url: string | undefined) => {
   if (!url) {
     console.warn('Attempted to open an empty url');
@@ -128,25 +160,7 @@ export const UrlPreviewCard = as<
   const [previewStatus, loadPreview] = useAsyncCallback(
     useCallback(() => {
       if (!ts && !bundle) return Promise.resolve(null);
-      if (urlPreview && ts) {
-        const remembered = getResultCache(mx).get(url);
-        if (remembered) {
-          return 'failed' in remembered
-            ? Promise.reject(new Error('preview previously refused'))
-            : Promise.resolve(remembered.data);
-        }
-        const clientCache = getClientCache(mx);
-        const cached = clientCache.get(url);
-        if (cached !== undefined) return cached;
-        const previewResult = mx?.getUrlPreview(url, ts);
-        if (!previewResult) return Promise.resolve(null);
-        clientCache.set(url, previewResult);
-        previewResult
-          .then((data) => rememberPreview(mx, url, { data }))
-          .catch(() => rememberPreview(mx, url, { failed: true }))
-          .finally(() => clientCache.delete(url));
-        return previewResult;
-      }
+      if (urlPreview && ts) return requestUrlPreview(mx, url, ts);
       return Promise.resolve(bundle);
     }, [ts, bundle, urlPreview, mx, url])
   );
@@ -158,48 +172,9 @@ export const UrlPreviewCard = as<
   }, [url, loadPreview]);
 
   const failed = previewStatus.status === AsyncStatus.Error || (settled && 'failed' in settled);
-  const pending = !failed && previewStatus.status !== AsyncStatus.Success && !settled;
-
-  // Hold the placeholder height until the card is off screen, so a shorter or refused
-  // preview does not pull the timeline up under the reader.
-  const rootRef = useRef<HTMLDivElement | null>(null);
-  const placeholderHeightRef = useRef<number>();
-  const [released, setReleased] = useState(false);
-
-  useLayoutEffect(() => {
-    if (pending && rootRef.current) placeholderHeightRef.current = rootRef.current.offsetHeight;
-  });
-
-  const reservedHeight = released ? undefined : placeholderHeightRef.current;
-
-  useEffect(() => {
-    const el = rootRef.current;
-    if (!el || released || pending || reservedHeight === undefined) return () => {};
-    const observer = new IntersectionObserver(
-      ([entry]) => {
-        if (entry && !entry.isIntersecting) setReleased(true);
-      },
-      // Well clear of the viewport: releasing at the edge is where virtua compensates least.
-      { rootMargin: '800px 0px' }
-    );
-    observer.observe(el);
-    return () => observer.disconnect();
-  }, [released, pending, reservedHeight]);
-
-  const setRootRef = useCallback(
-    (node: HTMLDivElement | null) => {
-      rootRef.current = node;
-      if (typeof ref === 'function') ref(node);
-      else if (ref) (ref as MutableRefObject<HTMLDivElement | null>).current = node;
-    },
-    [ref]
-  );
 
-  if (failed) {
-    return reservedHeight === undefined ? null : (
-      <div ref={rootRef} style={{ height: reservedHeight }} aria-hidden />
-    );
-  }
+  // Holding space for a card that never arrives leaves a permanent hole in the message.
+  if (failed) return null;
 
   const renderContent = (prev: IPreviewUrlResponse) => {
     const siteName = prev['og:site_name'];
@@ -440,46 +415,27 @@ export const UrlPreviewCard = as<
       </UrlPreviewContent>
     );
   } else {
-    // Same shape as a resolved card, so it does not grow when the preview lands.
+    // Kept midway between a refused preview and a text card: whichever lands, the height
+    // this was wrong by is as small as it can be.
     previewContent = (
       <Box grow="Yes" direction="Column" style={{ overflow: 'hidden', width: '100%' }}>
         <UrlPreviewContent style={{ minWidth: 0 }}>
           <LinePlaceholder style={{ maxWidth: toRem(160) }} />
           <LinePlaceholder style={{ maxWidth: toRem(240) }} />
-          <LinePlaceholder />
-          <LinePlaceholder style={{ maxWidth: toRem(280) }} />
         </UrlPreviewContent>
-        <Box
-          shrink="No"
-          alignItems="Center"
-          justifyContent="Center"
-          className={urlPreviewChrome.UrlPreviewMediaWell}
-          style={{
-            width: '100%',
-            maxHeight: toRem(linkPreviewImageMaxHeight),
-            aspectRatio: '16 / 9',
-          }}
-        >
-          <Spinner variant="Secondary" size="400" />
-        </Box>
       </Box>
     );
   }
   return (
-    <UrlPreview
-      {...props}
-      ref={setRootRef}
-      style={{
-        alignSelf: 'start',
-        minHeight: reservedHeight,
-      }}
-    >
+    <UrlPreview {...props} ref={ref} style={{ alignSelf: 'start' }}>
       {previewContent}
     </UrlPreview>
   );
 });
 
 export const UrlPreviewHolder = as<'div'>(({ children, ...props }, ref) => {
+  // An empty holder still contributes its top margin, e.g. when every widget kind is disabled.
+  const hasCard = Children.toArray(children).length > 0;
   const scrollRef = useRef<HTMLDivElement>(null);
   const innerBoxRef = useRef<HTMLDivElement>(null);
   const [canScrollLeft, setCanScrollLeft] = useState(false);
@@ -529,6 +485,8 @@ export const UrlPreviewHolder = as<'div'>(({ children, ...props }, ref) => {
     });
   };
 
+  if (!hasCard) return null;
+
   return (
     <Box
       direction="Column"

src/app/features/room/RoomTimeline.test.tsx

@@ -40,6 +40,7 @@ const {
     scrollTo: vi.fn<() => void>(),
     getItemOffset: () => 0,
     getItemSize: () => 100,
+    findItemIndex: () => 0,
   },
   timelineSync: {
     eventsLength: 1,

src/app/features/room/RoomTimeline.tsx

@@ -77,6 +77,7 @@ import { RoomMediaViewer } from '$components/image-viewer/RoomMediaViewer';
 import type { RoomMediaItem } from '$components/image-viewer/RoomMediaViewer';
 import type { IImageContent } from '$types/matrix/common';
 import { useTimelineRendererContext } from '$hooks/timeline/useTimelineRendererContext';
+import { useUrlPreviewPrefetch } from '$hooks/timeline/useUrlPreviewPrefetch';
 import { TimelineScrollingProvider, useScrollActivity } from '$hooks/useTimelineScrollActivity';
 import * as css from './RoomTimeline.css';
 import type { Persona } from '$app/persona';
@@ -1165,12 +1166,25 @@ export function RoomTimeline({
     };
   }, [eventId]);
 
+  const prefetchPreviews = useUrlPreviewPrefetch(mx, settings.showUrlPreview, processedEventsRef);
+
+  const prefetchAroundViewport = useCallback(() => {
+    const v = vListRef.current;
+    if (!v) return;
+    const { scrollOffset, viewportSize } = v;
+    prefetchPreviews(v.findItemIndex(scrollOffset), v.findItemIndex(scrollOffset + viewportSize));
+  }, [prefetchPreviews]);
+
+  useEffect(prefetchAroundViewport, [prefetchAroundViewport, timelineSync.eventsLength]);
+
   const handleVListScroll = useCallback(
     (offset: number) => {
       notifyScroll();
       const v = vListRef.current;
       if (!v) return;
 
+      prefetchAroundViewport();
+
       const distanceFromBottom = v.scrollSize - offset - v.viewportSize;
       syncAtBottom(offset);
 
@@ -1218,7 +1232,7 @@ export function RoomTimeline({
         void timelineSyncRef.current.handleTimelinePagination(false);
       }
     },
-    [eventId, notifyScroll, syncAtBottom]
+    [eventId, notifyScroll, syncAtBottom, prefetchAroundViewport]
   );
   const handleVListScrollEnd = useCallback(() => {
     if (!timelineSyncRef.current.focusItem?.scrollTo) return;

src/app/hooks/timeline/useUrlPreviewPrefetch.test.ts

@@ -0,0 +1,43 @@
+import { describe, expect, it } from 'vitest';
+import type { MatrixEvent } from '$types/matrix-sdk';
+import { previewableLinks, rowsByDistance } from './useUrlPreviewPrefetch';
+
+const textEvent = (body: string, msgtype = 'm.text'): MatrixEvent =>
+  ({ getContent: () => ({ msgtype, body }) }) as unknown as MatrixEvent;
+
+describe('previewableLinks', () => {
+  it('finds every http(s) link in the body', () => {
+    expect(previewableLinks(textEvent('see https://a.example/x and http://b.example'))).toEqual([
+      'https://a.example/x',
+      'http://b.example',
+    ]);
+  });
+
+  it('skips permalinks, which never carry a preview', () => {
+    expect(previewableLinks(textEvent('hi https://matrix.to/#/@a:b.example'))).toEqual([]);
+  });
+
+  it('skips message types that render no preview', () => {
+    expect(previewableLinks(textEvent('https://a.example', 'm.image'))).toEqual([]);
+  });
+
+  it('tolerates a body that is missing or not a string', () => {
+    expect(previewableLinks({ getContent: () => ({ msgtype: 'm.text' }) } as MatrixEvent)).toEqual(
+      []
+    );
+  });
+
+  it('leaves a trailing bracket out of the url', () => {
+    expect(previewableLinks(textEvent('(https://a.example/x)'))).toEqual(['https://a.example/x']);
+  });
+});
+
+describe('rowsByDistance', () => {
+  it('walks outward from both edges of the rendered window', () => {
+    expect(rowsByDistance(5, 7, 100, 3)).toEqual([4, 8, 3, 9, 2, 10]);
+  });
+
+  it('stays inside the row range', () => {
+    expect(rowsByDistance(1, 8, 10, 3)).toEqual([0, 9]);
+  });
+});

src/app/hooks/timeline/useUrlPreviewPrefetch.ts

@@ -0,0 +1,95 @@
+import type { MutableRefObject } from 'react';
+import { useCallback, useRef } from 'react';
+import type { MatrixClient, MatrixEvent } from '$types/matrix-sdk';
+import { MsgType } from '$types/matrix-sdk';
+import { prefetchUrlPreview } from '$components/url-preview';
+import { testMatrixTo } from '$plugins/matrix-to';
+import { testMatrixUri } from '$plugins/matrix-uri';
+import type { ProcessedEvent } from './useProcessedTimeline';
+
+// Rows of runway ahead of the rendered window for a preview to resolve in.
+const PREFETCH_ROWS = 40;
+
+// A whole pass at once saturates the per-host cap and delays the nearest rows; six matches it.
+const MAX_IN_FLIGHT = 6;
+
+const LINK_REGEX = /https?:\/\/[^\s<>"')\]]+/g;
+
+const PREVIEWABLE_MSGTYPES = new Set<string>([MsgType.Text, MsgType.Notice, MsgType.Emote]);
+
+export const previewableLinks = (mEvent: MatrixEvent): string[] => {
+  const content = mEvent.getContent();
+  if (!PREVIEWABLE_MSGTYPES.has(content.msgtype ?? '')) return [];
+  const body = typeof content.body === 'string' ? content.body : '';
+  const links = body.match(LINK_REGEX);
+  if (!links) return [];
+  return links.filter((url) => !testMatrixTo(url) && !testMatrixUri(url));
+};
+
+/** Row indices around the rendered window, nearest to it first. */
+export const rowsByDistance = (
+  startIndex: number,
+  endIndex: number,
+  rowCount: number,
+  reach: number
+): number[] => {
+  const indices: number[] = [];
+  for (let step = 1; step <= reach; step += 1) {
+    const above = startIndex - step;
+    const below = endIndex + step;
+    if (above >= 0) indices.push(above);
+    if (below < rowCount) indices.push(below);
+  }
+  return indices;
+};
+
+export const useUrlPreviewPrefetch = (
+  mx: MatrixClient,
+  enabled: boolean,
+  eventsRef: MutableRefObject<ProcessedEvent[]>
+) => {
+  const lastRangeRef = useRef('');
+  const queueRef = useRef<{ url: string; ts: number }[]>([]);
+  const queuedRef = useRef(new Set<string>());
+  const inFlightRef = useRef(0);
+
+  const pump = useCallback(() => {
+    while (inFlightRef.current < MAX_IN_FLIGHT) {
+      const next = queueRef.current.shift();
+      if (!next) return;
+      queuedRef.current.delete(next.url);
+      inFlightRef.current += 1;
+      prefetchUrlPreview(mx, next.url, next.ts).finally(() => {
+        inFlightRef.current -= 1;
+        pump();
+      });
+    }
+  }, [mx]);
+
+  return useCallback(
+    (startIndex: number, endIndex: number) => {
+      if (!enabled) return;
+      const events = eventsRef.current;
+      // Row count is in the key so a page prepended without the reader moving still runs.
+      const key = `${startIndex}:${endIndex}:${events.length}`;
+      if (key === lastRangeRef.current) return;
+      lastRangeRef.current = key;
+
+      queueRef.current = [];
+      queuedRef.current.clear();
+
+      for (const index of rowsByDistance(startIndex, endIndex, events.length, PREFETCH_ROWS)) {
+        const mEvent = events[index]?.mEvent;
+        if (!mEvent) continue;
+        const ts = mEvent.getTs();
+        for (const url of previewableLinks(mEvent)) {
+          if (queuedRef.current.has(url)) continue;
+          queuedRef.current.add(url);
+          queueRef.current.push({ url, ts });
+        }
+      }
+      pump();
+    },
+    [enabled, eventsRef, pump]
+  );
+};

tests/e2e/pages/AppShell.ts

@@ -19,7 +19,7 @@ export class AppShell {
   }
 
   async sendTextMessage(text: string): Promise<void> {
-    await this.page.locator('div[data-slate-editor="true"]').last().click();
+    await this.page.locator('[data-editable-name="RoomInput"]').last().click();
     await this.page.keyboard.type(text);
     await this.page.keyboard.press('Enter');
   }