Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: image loading handlers #6581

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
17 changes: 6 additions & 11 deletions packages/lib-classifier/src/hooks/useSubjectImage.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ export default function useSubjectImage({ src, onReady, onError }) {

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
Expand All @@ -20,14 +22,7 @@ export default function useSubjectImage({ src, onReady, onError }) {
const target = { clientHeight, clientWidth, naturalHeight, naturalWidth }
onReady({ target })
}
}, [img, onReady, subjectImage])

useEffect(function logError() {
if (!loading && error) {
console.error(error)
onError(error)
}
}, [error, loading, onError])
}

return { img, error, loading, subjectImage }
}
1 change: 1 addition & 0 deletions packages/lib-react-components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
11 changes: 10 additions & 1 deletion packages/lib-react-components/src/hooks/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.'/>
```
Expand Down
54 changes: 37 additions & 17 deletions packages/lib-react-components/src/hooks/useProgressiveImage.js
Original file line number Diff line number Diff line change
@@ -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() {
Expand All @@ -43,5 +63,5 @@ export default function useProgressiveImage({
}
}, [src])

return { img, error, loading }
return { img: imgRef.current, error, loading }
}
Loading