refactor: image loading handlers (#6581)

* refactor: image loading handlers

Some changes to clean up image loading code, and speed up subject image loading.
- lib-classifier: remove `useEffect` from `useSubjectImage`, so that image loading code runs on image load, rather than waiting for the next React render cycle.
- lib-react-components: when there's no delay for `useProgressiveImage`, run the `onLoad` handler immediately, rather than waiting for the next event cycle.
- refactor `onLoad` event handlers to use `load` events, rather than `Image` objects. This brings the event handlers in line with the DOM standard.

* add mock load events to classifier tests

* fix: add frameIndex to onSubjectReady callback

* add frame index to ImageAndText viewer

Author: eatyourgreens

packages/lib-classifier/src/components/Classifier/Classifier.spec.js

@@ -38,15 +38,23 @@ describe('Components > Classifier', function () {
     constructor () {
       this.naturalHeight = 1000
       this.naturalWidth = 500
-      setTimeout(() => this.onload(), 500)
+      const fakeLoadEvent = {
+        ...new Event('load'),
+        target: this
+      }
+      setTimeout(() => this.onload(fakeLoadEvent), 500)
     }
   }
 
   class MockSlowImage {
     constructor () {
       this.naturalHeight = 1000
       this.naturalWidth = 500
-      setTimeout(() => this.onload(), 5000)
+      const fakeLoadEvent = {
+        ...new Event('load'),
+        target: this
+      }
+      setTimeout(() => this.onload(fakeLoadEvent), 5000)
     }
   }
 

packages/lib-classifier/src/components/Classifier/Classifier.workflows.spec.js

@@ -76,7 +76,11 @@ function testWorkflow(workflowSnapshot, workflowStrings) {
       constructor () {
         this.naturalHeight = 1000
         this.naturalWidth = 500
-        setTimeout(() => this.onload(), 500)
+        const fakeLoadEvent = {
+          ...new Event('load'),
+          target: this
+        }
+        setTimeout(() => this.onload(fakeLoadEvent), 500)
       }
     })
 

packages/lib-classifier/src/components/Classifier/ClassifierContainer.spec.js

@@ -33,8 +33,12 @@ describe('components > ClassifierContainer', function () {
     constructor () {
       this.naturalHeight = 1000
       this.naturalWidth = 500
+      const fakeLoadEvent = {
+        ...new Event('load'),
+        target: this
+      }
       setTimeout(() => {
-        this.onload()
+        this.onload(fakeLoadEvent)
       }, 500)
     }
   }

packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/FlipbookViewer/FlipbookViewer.js

@@ -32,12 +32,13 @@ const FlipbookViewer = ({
   const [currentFrame, setCurrentFrame] = useState(defaultFrame)
   const [playing, setPlaying] = useState(false)
   const [dragMove, setDragMove] = useState()
-  /** This initializes an image element from the subject's defaultFrame src url.
+  /** This initializes an image element from the subject's currentFrame src url.
    * We do this so the SVGPanZoom has dimensions of the subject image.
-   * We're assuming all frames in one subject have the same dimensions. */
-  const defaultFrameLocation = subject ? subject.locations[defaultFrame] : null
+   */
+  const currentFrameLocation = subject ? subject.locations[currentFrame] : null
   const { img, error, loading, subjectImage } = useSubjectImage({
-    src: defaultFrameLocation.url,
+    frame: currentFrame,
+    src: currentFrameLocation.url,
     onReady,
     onError
   })

packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/ImageAndTextViewer/ImageAndTextViewerContainer.js

@@ -48,13 +48,15 @@ function ImageAndTextViewerContainer ({
         {(type === 'text')
           ? (
             <SingleTextViewer
+              frame={frame}
               height={dimensions[0]?.clientHeight ? `${dimensions[0]?.clientHeight}px` : ''}
               onError={onError}
               onReady={onReady}
             />)
           : (
             <SingleImageViewer
               enableInteractionLayer={false}
+              frame={frame}
               loadingState={loadingState}
               onError={onError}
               onReady={onReady}

packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/MultiFrameViewer/MultiFrameViewerContainer.js

@@ -77,6 +77,7 @@ function MultiFrameViewerContainer({
   // TODO: replace this with a better function to parse the image location from a subject.
   const imageLocation = subject ? subject.locations[frame] : null
   const { img, error, loading, subjectImage } = useSubjectImage({
+    frame,
     src: imageLocation?.url,
     onReady,
     onError

packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/MultiFrameViewer/MultiFrameViewerContainer.spec.js

@@ -26,7 +26,11 @@ describe('Component > MultiFrameViewerContainer', function () {
     constructor () {
       this.naturalHeight = height
       this.naturalWidth = width
-      setTimeout(() => this.onload(), DELAY)
+      const fakeLoadEvent = {
+        ...new Event('load'),
+        target: this
+      }
+      setTimeout(() => this.onload(fakeLoadEvent), DELAY)
     }
   }
 

packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/SeparateFramesViewer/components/SeparateFrame/SeparateFrame.js

@@ -32,6 +32,7 @@ const SeparateFrame = ({
   onReady = DEFAULT_HANDLER
 }) => {
   const { img, error, loading, subjectImage } = useSubjectImage({
+    frame,
     src: frameUrl,
     onReady,
     onError

packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/SingleImageViewer/SingleImageViewerContainer.spec.js

@@ -24,7 +24,11 @@ describe('Component > SingleImageViewerContainer', function () {
     constructor () {
       this.naturalHeight = height
       this.naturalWidth = width
-      setTimeout(() => this.onload(), DELAY)
+      const fakeLoadEvent = {
+        ...new Event('load'),
+        target: this
+      }
+      setTimeout(() => this.onload(fakeLoadEvent), DELAY)
     }
   }
 

packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/SingleTextViewer/SingleTextViewerContainer.js

@@ -49,6 +49,7 @@ function DEFAULT_HANDLER() {
 }
 
 export function SingleTextViewerContainer ({
+  frame = 0,
   height = '',
   latest,
   loadingState = asyncStates.initialized,
@@ -57,6 +58,7 @@ export function SingleTextViewerContainer ({
   subject = defaultSubject
 }) {
   const { data, error, loading } = useSubjectText({
+    frame,
     subject,
     onReady,
     onError

packages/lib-classifier/src/hooks/Readme.md

@@ -104,7 +104,7 @@ A custom hook that fetches an image from a URL, with a ref to the image's DOM no
 Usage:
 ```jsx
 // img is a DOM img. subjectImage is a React ref to the element that displays the image.
-const { img, error, loading, subjectImage } = useSubjectImage({ src, onReady, onError })
+const { img, error, loading, subjectImage } = useSubjectImage({ frame, src, onReady, onError })
 
 if (loading) {
   return <p>The image is still loading.</p>

packages/lib-classifier/src/hooks/useSubjectImage.js

@@ -1,33 +1,28 @@
 import { useEffect, useRef } from 'react'
 import { useProgressiveImage } from '@zooniverse/react-components/hooks'
 
-export default function useSubjectImage({ src, onReady, onError }) {
+export default function useSubjectImage({ frame = 0, src, onReady, onError }) {
   const subjectImage = useRef()
 
   const { img, error, loading } = useProgressiveImage({
     placeholderSrc: '',
-    src
+    src,
+    onLoad,
+    onError
   })
 
 
-  useEffect(function onImageLoad() {
-    const { naturalHeight, naturalWidth, src } = img
+  function onLoad(event) {
+    const { naturalHeight, naturalWidth, src } = event.target
     if (src !== '' ) {
       const svgImage = subjectImage.current
       const { width: clientWidth, height: clientHeight } = svgImage
         ? svgImage.getBoundingClientRect()
         : {}
       const target = { clientHeight, clientWidth, naturalHeight, naturalWidth }
-      onReady({ target })
+      onReady({ target }, frame)
     }
-  }, [img, onReady, subjectImage])
-
-  useEffect(function logError() {
-    if (!loading && error) {
-      console.error(error)
-      onError(error)
-    }
-  }, [error, loading, onError])
+  }
 
   return { img, error, loading, subjectImage }
 }

packages/lib-classifier/src/hooks/useSubjectText.js

@@ -31,6 +31,8 @@ const DEFAULT_HANDLER = function () {
   A custom hook to load text data subjects from Panoptes
 */
 export default function useSubjectText({
+  /** location frame index */
+  frame = 0,
   /** Panoptes subject */
   subject,
   /** on data ready callback */
@@ -44,7 +46,7 @@ export default function useSubjectText({
   useEffect(function onSubjectChange() {
     function onLoad(rawData) {
       setData(rawData)
-      onReady()
+      onReady(null, frame)
     }
 
     async function handleSubject() {

packages/lib-classifier/src/store/SubjectViewerStore/SubjectViewerStore.js

@@ -115,15 +115,15 @@ const SubjectViewer = types
         self.loadingState = asyncStates.error
       },
 
-      onSubjectReady (event) {
+      onSubjectReady (event, frameIndex = 0) {
         const { target = {} } = event || {}
         const {
           clientHeight = 0,
           clientWidth = 0,
           naturalHeight = 0,
           naturalWidth = 0
         } = target || {}
-        self.dimensions.push({ clientHeight, clientWidth, naturalHeight, naturalWidth })
+        self.dimensions[frameIndex] = { clientHeight, clientWidth, naturalHeight, naturalWidth }
         self.rotation = 0
         self.loadingState = asyncStates.success
       },

packages/lib-react-components/CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - PlainButton has text-decoration underline on hover even for links.
 - ZooFooter links and labels updated to reflect newly launched FEM pages.
 - Updated styling in ProjectCard's badge component.
+- `useProgressiveImage`: run `onLoad` callbacks immediately when `delay` is 0. Add optional `onLoad` and `onError` event handlers for image `load` events.
 
 ### Fixed
 

packages/lib-react-components/src/Media/components/ThumbnailImage/ThumbnailImage.spec.js

@@ -12,7 +12,11 @@ describe('ThumbnailImage', function () {
     constructor () {
       this.naturalHeight = 200
       this.naturalWidth = 400
-      setTimeout(() => this.onload(), 0)
+      const fakeLoadEvent = {
+        ...new Event('load'),
+        target: this
+      }
+      setTimeout(() => this.onload(fakeLoadEvent), 0)
     }
   }
 

packages/lib-react-components/src/hooks/Readme.md

@@ -47,8 +47,17 @@ const { data: user, error, isLoading } = usePanoptesUser()
 Use a placeholder while an image downloads.
 
 ```jsx
+function onLoad(event) {
+  console.log('image loaded: ', event.target)
+}
+
+function onError(error) {
+  console.warn('loading failed')
+  console.error(error)
+}
+
 const src = 'https://panoptes-uploads.zooniverse.org/production/subject_location/66094a64-8823-4314-8ef4-1ee228e49470.jpeg'
-const { img, error, loading } = useProgressiveImage({ delay: 0, src })
+const { img, error, loading } = useProgressiveImage({ delay: 0, src, onLoad, onError })
 
 return <img src={img.src} alt='This is an example of an image with a placeholder.'/>
 ```

packages/lib-react-components/src/hooks/useProgressiveImage.js

@@ -1,40 +1,60 @@
-import { useEffect, useState } from 'react'
+import { useEffect, useRef, useState } from 'react'
 
 const defaultPlaceholder = {
   naturalHeight: 600,
   naturalWidth: 800,
   src: ''
 }
 
+const DEFAULT_HANDLER = () => false
+
+function preloadImage({
+  src,
+  onLoad = DEFAULT_HANDLER,
+  onError = DEFAULT_HANDLER
+}) {
+  const { Image } = window
+  const img = new Image()
+  img.onload = onLoad
+  img.onerror = onError
+  img.src = src
+}
+
 export default function useProgressiveImage({
   delay = 0,
   placeholderSrc = '',
-  src
+  src,
+  onLoad = DEFAULT_HANDLER,
+  onError = DEFAULT_HANDLER
 }) {
   const placeholder = {
     ...defaultPlaceholder,
     src: placeholderSrc
   }
   const [loading, setLoading] = useState(true)
-  const [img, setImg] = useState(placeholder)
+  const imgRef = useRef(placeholder)
   const [error, setError] = useState(null)
 
+  function onImageLoad(event) {
+    imgRef.current = event.target
+    onLoad(event)
+    setLoading(false)
+  }
+
+  function onImageError(error) {
+    onError(error)
+    setError(error)
+    setLoading(false)
+  }
+
   function fetchImage() {
-    const { Image } = window
-    const img = new Image()
-    img.onload = () => {
-      setTimeout(() => {
-        setImg(img)
-        setLoading(false)
-      }, delay)
-    }
-    img.onerror = (error) => {
-      setError(error)
-      setLoading(false)
-    }
     setLoading(true)
     setError(null)
-    img.src = src
+    preloadImage({
+      src,
+      onLoad: delay ? event => setTimeout(onImageLoad, delay, event) : onImageLoad,
+      onError: onImageError
+    })
   }
 
   useEffect(function onNewImage() {
@@ -43,5 +63,5 @@ export default function useProgressiveImage({
     }
   }, [src])
 
-  return { img, error, loading }
+  return { img: imgRef.current, error, loading }
 }