refactor(cuesheet): reserve the space an image will actually take

We were remembering the aspect ratio and reserving the full width of the
column, which over-reserves for an image smaller than the column: the row
would settle to a smaller size once the image was shown.

We now remember the size of the image and express the reservation in CSS
as min(100%, width), which is what the image itself resolves to at any
column width. Column sizes are applied as CSS variables and do not
re-render the cells, so the reservation follows a resize on its own.

Also groups the tests around the behaviour they describe.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CKXcDrZoQXbJpaXiLi1aff

apps/client/src/common/utils/__tests__/imageDimensions.test.ts

@@ -0,0 +1,26 @@
+import { getRememberedDimensions, rememberDimensions } from '../imageDimensions';
+
+/** stand-in for a loaded HTMLImageElement */
+function makeImage(naturalWidth: number, naturalHeight: number) {
+  return { naturalWidth, naturalHeight } as HTMLImageElement;
+}
+
+test('We remember the size of an image, so that we can reserve its space when it comes back', () => {
+  expect(getRememberedDimensions('http://ontime.local/unseen.png')).toBe(null);
+
+  rememberDimensions('http://ontime.local/image.png', makeImage(1920, 1080));
+  expect(getRememberedDimensions('http://ontime.local/image.png')).toMatchObject({ width: 1920, height: 1080 });
+
+  // an image which failed to load has no size to offer
+  rememberDimensions('http://ontime.local/broken.png', makeImage(0, 0));
+  expect(getRememberedDimensions('http://ontime.local/broken.png')).toBe(null);
+});
+
+test('We keep the most recently seen images, older entries are forgotten', () => {
+  for (let i = 0; i < 600; i++) {
+    rememberDimensions(`http://ontime.local/${i}.png`, makeImage(100, 50));
+  }
+
+  expect(getRememberedDimensions('http://ontime.local/0.png')).toBe(null);
+  expect(getRememberedDimensions('http://ontime.local/599.png')).toMatchObject({ width: 100, height: 50 });
+});

apps/client/src/common/utils/imageDimensions.ts

@@ -0,0 +1,49 @@
+/**
+ * Images in the cuesheet live inside a virtualised table:
+ * rows are unmounted when they leave the viewport and mounted again when they come back.
+ * A re-mounted image has no dimensions until it is available,
+ * which makes the row change height and the table shift under the user.
+ *
+ * We remember the size of the images we have already seen
+ * so that we can reserve the space they will take.
+ * This only holds two numbers per image: we leave the image data itself to the browser cache,
+ * which knows better than us when memory should be released.
+ */
+
+export interface ImageDimensions {
+  width: number;
+  height: number;
+}
+
+/** how many sizes we remember, this is only a few bytes per entry */
+const maxSize = 500;
+
+const dimensions = new Map<string, ImageDimensions>();
+
+/**
+ * @returns the size of a previously loaded image, if we have seen it before
+ */
+export function getRememberedDimensions(src: string): ImageDimensions | null {
+  return dimensions.get(src) ?? null;
+}
+
+/**
+ * Records the size of a loaded image
+ */
+export function rememberDimensions(src: string, image: HTMLImageElement) {
+  if (image.naturalHeight === 0) {
+    return;
+  }
+
+  // the map iteration order is our LRU queue, re-adding the entry marks it as recently used
+  dimensions.delete(src);
+  dimensions.set(src, { width: image.naturalWidth, height: image.naturalHeight });
+
+  while (dimensions.size > maxSize) {
+    const oldest = dimensions.keys().next();
+    if (oldest.done) {
+      return;
+    }
+    dimensions.delete(oldest.value);
+  }
+}

apps/client/src/views/cuesheet/__tests__/EditableImage.test.ts

@@ -0,0 +1,18 @@
+import { isValidImageSource } from '../cuesheet-table/cuesheet-table-elements/EditableImage';
+
+test('An image is referenced by link, anything else is rejected', () => {
+  const testCases = [
+    { value: 'https://example.com/image.png', isValid: true },
+    { value: 'http://example.com/image.png', isValid: true },
+    // a file is local to the machine running ontime, it would not resolve for the clients we serve
+    { value: '/user/image.png', isValid: false },
+    { value: 'file:///Users/me/image.png', isValid: false },
+    { value: 'C:\\images\\image.png', isValid: false },
+    // values which do not describe a location we can reach
+    { value: 'www.example.com/image.png', isValid: false },
+    { value: 'https://', isValid: false },
+    { value: 'some text', isValid: false },
+  ];
+
+  testCases.forEach((t) => expect(isValidImageSource(t.value)).toBe(t.isValid));
+});

apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/EditableImage.module.scss

@@ -5,6 +5,19 @@
   &:not(:read-only):hover::placeholder {
     opacity: 1;
   }
+
+  &[data-invalid] {
+    outline: 1px solid $red-500;
+  }
+}
+
+/** feedback on a value we cannot use, either rejected or failed to load */
+.message {
+  display: block;
+  padding: 0.25rem 0;
+  color: $red-500;
+  font-size: calc(1rem - 3px);
+  line-height: 1.2;
 }
 
 .imageCell {

apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/EditableImage.tsx

@@ -1,27 +1,54 @@
-import { memo } from 'react';
+import { memo, useState } from 'react';
 
 import Button from '../../../../common/components/buttons/Button';
 import Input from '../../../../common/components/input/input/Input';
+import { getRememberedDimensions, rememberDimensions } from '../../../../common/utils/imageDimensions';
 
 import style from './EditableImage.module.scss';
 
 interface EditableImageProps {
   initialValue: string;
+  fieldLabel: string;
   readOnly?: boolean;
   updateValue: (newValue: string) => void;
 }
 
 export default memo(EditableImage);
 
-function EditableImage({ initialValue, readOnly, updateValue }: EditableImageProps) {
+/**
+ * Images are referenced by link: anything local to the machine running ontime
+ * would not resolve for the clients we serve the cuesheet to
+ */
+export function isValidImageSource(value: string): boolean {
+  try {
+    const url = new URL(value);
+    return url.protocol === 'http:' || url.protocol === 'https:';
+  } catch {
+    return false;
+  }
+}
+
+function EditableImage({ initialValue, fieldLabel, readOnly, updateValue }: EditableImageProps) {
+  const [isRejected, setIsRejected] = useState(false);
+  /** we keep track of the source itself, so that the state follows the value being shown */
+  const [failedSource, setFailedSource] = useState<string | null>(null);
+  const [loadedSource, setLoadedSource] = useState<string | null>(null);
+
   const handleUpdate = (newValue: string) => {
-    if (newValue === initialValue) {
+    const value = newValue.trim();
+
+    if (value === initialValue) {
+      setIsRejected(false);
       return;
     }
-    if (newValue !== '' && !newValue.startsWith('http')) {
+
+    if (value !== '' && !isValidImageSource(value)) {
+      setIsRejected(true);
       return;
     }
-    updateValue(newValue);
+
+    setIsRejected(false);
+    updateValue(value);
   };
 
   const openInNewTab = () => {
@@ -36,22 +63,42 @@ function EditableImage({ initialValue, readOnly, updateValue }: EditableImagePro
 
   if (!initialValue) {
     return (
-      <Input
+      <>
-        variant='ghosted'
+        <Input
-        className={style.imageInput}
+          variant='ghosted'
-        fluid
+          className={style.imageInput}
-        placeholder='Paste image URL'
+          fluid
-        onBlur={(event) => handleUpdate(event.currentTarget.value)}
+          placeholder='Paste image URL'
-        onKeyDown={(event) => {
+          data-invalid={isRejected || undefined}
-          if (event.key === 'Enter') {
+          onChange={() => setIsRejected(false)}
-            handleUpdate(event.currentTarget.value);
+          onBlur={(event) => handleUpdate(event.currentTarget.value)}
-          }
+          onKeyDown={(event) => {
-        }}
+            if (event.key === 'Enter') {
-        defaultValue={initialValue}
+              handleUpdate(event.currentTarget.value);
-      />
+            }
+          }}
+        />
+        {isRejected && <span className={style.message}>Images are referenced by link (https://...)</span>}
+      </>
     );
   }
 
+  /**
+   * The cuesheet is virtualised: rows are unmounted once they leave the viewport.
+   * When the row comes back, we reserve the space the image took
+   * so that the table does not shift while the browser makes it available.
+   * The reservation is given in CSS so that it follows the column being resized,
+   * the same way the image itself does once it is shown.
+   */
+  const knownDimensions = getRememberedDimensions(initialValue);
+  const isLoaded = loadedSource === initialValue;
+  const reservedSpace = knownDimensions
+    ? {
+        aspectRatio: knownDimensions.width / knownDimensions.height,
+        width: `min(100%, ${knownDimensions.width}px)`,
+      }
+    : undefined;
+
   return (
     <div className={style.imageCell}>
       {!readOnly && (
@@ -62,7 +109,22 @@ function EditableImage({ initialValue, readOnly, updateValue }: EditableImagePro
           </Button>
         </div>
       )}
-      {Boolean(initialValue) && <img loading='lazy' src={initialValue} className={style.image} />}
+      {failedSource === initialValue ? (
+        <span className={style.message}>Could not load image</span>
+      ) : (
+        <img
+          src={initialValue}
+          alt={fieldLabel}
+          className={style.image}
+          onLoad={(event) => {
+            rememberDimensions(initialValue, event.currentTarget);
+            setLoadedSource(initialValue);
+          }}
+          onError={() => setFailedSource(initialValue)}
+          /** until the image is available, we reserve the space it took the last time we saw it */
+          style={isLoaded ? undefined : reservedSpace}
+        />
+      )}
     </div>
   );
 }

apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/cuesheetColsFactory.tsx

@@ -175,7 +175,14 @@ function LazyImage({ row, column, table }: CuesheetCellContext) {
 
   const canWrite = column.columnDef.meta?.canWrite;
   const initialValue = event.custom[column.id];
-  return <EditableImage initialValue={initialValue} updateValue={update} readOnly={!canWrite} />;
+  return (
+    <EditableImage
+      initialValue={initialValue}
+      fieldLabel={getColumnLabel(column)}
+      updateValue={update}
+      readOnly={!canWrite}
+    />
+  );
 }
 
 function MakeSingleLineField({ row, column, table }: CuesheetCellContext) {