From 3d15f9fc8ea2954b4838c8242fda2988ea132b60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sat, 29 Aug 2026 09:35:42 +0100 Subject: [PATCH] the spinner stops sticking on cached images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit my own fix from an hour ago caused this. it reset a boolean in an effect on `src`, and that has a race which a CACHED image wins every time: render with the new src -> browser has it already, fires onLoad -> loaded = true effect runs (after paint) -> loaded = false onLoad never fires again -> spinner forever so the files that stuck were exactly the ones that were fastest, which is why it read as random. now it is DERIVED rather than reset: state holds which src has loaded, and `loaded` is that compared to the current one. false the instant `src` changes, true the moment that src's own load fires, and no window where the two can be written out of order. there is nothing left to race. zoom, rotation and pan stay in an effect. nothing else writes them during a load, and being one render late is invisible — the hazard was only ever on the flag the browser also writes. Co-Authored-By: Claude Opus 5 Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043 --- .../FileViewer/renderers/ImageRenderer.tsx | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/workspaces/officerdev/src/apps/FileViewer/renderers/ImageRenderer.tsx b/src/workspaces/officerdev/src/apps/FileViewer/renderers/ImageRenderer.tsx index 88724e8e..f1dda488 100644 --- a/src/workspaces/officerdev/src/apps/FileViewer/renderers/ImageRenderer.tsx +++ b/src/workspaces/officerdev/src/apps/FileViewer/renderers/ImageRenderer.tsx @@ -16,8 +16,22 @@ export const ImageRenderer = ({ src, fileName }: ImageRendererProps) => { const containerRef = useRef(null); const [zoom, setZoom] = useState(1); const [rotation, setRotation] = useState(0); - const [loaded, setLoaded] = useState(false); - const [error, setError] = useState(false); + /* + WHICH src has loaded, not whether one has. + + The first fix reset a boolean in an effect on `src`, and that has a race a cached image wins every + time: the effect runs AFTER render, so the browser fires `onLoad` for an image already in cache + before the effect gets to clear the flag — and then the effect clears it, and `onLoad` never comes + again. The spinner stayed forever on exactly the files that were fastest. + + Comparing the loaded src to the current one has no ordering at all. It is false the instant `src` + changes, true the moment that src's own load fires, and there is no window in which the two can be + written in the wrong order. + */ + const [loadedSrc, setLoadedSrc] = useState(null); + const [errorSrc, setErrorSrc] = useState(null); + const loaded = loadedSrc === src; + const error = errorSrc === src; const [dragging, setDragging] = useState(false); const [offset, setOffset] = useState({ x: 0, y: 0 }); const dragStart = useRef({ x: 0, y: 0, ox: 0, oy: 0 }); @@ -90,9 +104,12 @@ export const ImageRenderer = ({ src, fileName }: ImageRendererProps) => { onto the next photograph means arriving somewhere nobody asked to be, in the corner of a picture they have not seen yet. */ + /* + Zoom, rotation and pan are a view of ONE image, so they reset when the image changes. These stay in + an effect because they have no race to lose: nothing else writes them during a load, and being one + render late is invisible. + */ useEffect(() => { - setLoaded(false); - setError(false); setZoom(1); setRotation(0); setOffset({ x: 0, y: 0 }); @@ -125,8 +142,8 @@ export const ImageRenderer = ({ src, fileName }: ImageRendererProps) => { {fileName} setLoaded(true)} - onError={() => setError(true)} + onLoad={() => setLoadedSrc(src)} + onError={() => setErrorSrc(src)} className="transition-transform duration-150 select-none" draggable={false} style={{