Skip to content

Commit 6d29d73

Browse files
author
tonylepmets
authored
Fix thumbnail reuse (#86)
* Fix thumbnail reuse in PDP when no media present Without any media present on component mount the only image will be the thumbnail itself. Make sure the `load` event listener is only added to the first image that is not the thumbnail. * Remove unused onLoad prop This didn't work with neither magnified or regular image components. Thumbnail reuse works through the image selector ref. * Bump version
1 parent e6ab35f commit 6d29d73

File tree

4 files changed

+25
-7
lines changed

4 files changed

+25
-7
lines changed

package-lock.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "react-storefront",
3-
"version": "8.3.0",
3+
"version": "8.3.1",
44
"description": "Build and deploy e-commerce progressive web apps (PWAs) in record time.",
55
"module": "./index.js",
66
"license": "Apache-2.0",

src/carousel/MediaCarousel.js

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import MagnifyHint from './MagnifyHint'
1313
import CarouselThumbnails from './CarouselThumbnails'
1414
import get from 'lodash/get'
1515

16+
const THUMBNAIL_IMAGE_ID = '__rsf-placeholder-thumbnail'
17+
1618
export const styles = theme => ({
1719
/**
1820
* Styles applied to the root component.
@@ -198,18 +200,26 @@ function MediaCarousel(props) {
198200
})
199201

200202
useEffect(() => {
203+
if (!ref.current || imagesLoaded || !thumbnail) return
201204
const firstImage = ref.current.querySelector('img')
202-
203-
if (firstImage) {
205+
if (firstImage && firstImage.id !== THUMBNAIL_IMAGE_ID) {
204206
firstImage.addEventListener('load', onFullSizeImagesLoaded)
205207
return () => firstImage.removeEventListener('load', onFullSizeImagesLoaded)
206208
}
207-
}, [])
209+
}, [media, imagesLoaded, thumbnail])
208210

209211
const belowAdornments = []
210212

211213
if (thumbnail && !imagesLoaded) {
212-
belowAdornments.push(<Image key="thumbnail" className={styles.thumbnail} fill {...thumbnail} />)
214+
belowAdornments.push(
215+
<Image
216+
id={THUMBNAIL_IMAGE_ID}
217+
key="thumbnail"
218+
className={styles.thumbnail}
219+
fill
220+
{...thumbnail}
221+
/>,
222+
)
213223
}
214224

215225
if (media && media.full && media.full.some(item => item.magnify)) {
@@ -278,7 +288,6 @@ function MediaCarousel(props) {
278288
media.full.map((item, i) => (
279289
<Fill height="100%" key={i}>
280290
<MediaComponent
281-
onLoad={i === 0 ? onFullSizeImagesLoaded : null}
282291
magnifyProps={magnifyProps}
283292
{...item}
284293
src={get(item, 'magnify.src', item.src)}

test/carousel/MediaCarousel.test.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,15 @@ describe('MediaCarousel', () => {
228228
loadMock.mockClear()
229229
})
230230

231+
it('should not add img listener if the only image is the preview thumbnail', async () => {
232+
const loadMock = jest.spyOn(HTMLImageElement.prototype, 'addEventListener')
233+
234+
wrapper = mount(<MediaCarousel thumbnails={false} thumbnail={{ test: 'test' }} />)
235+
236+
expect(loadMock).not.toBeCalled()
237+
loadMock.mockClear()
238+
})
239+
231240
it('should have timeout on magnifying carousel', async () => {
232241
wrapper = mount(<MediaCarousel media={media} />)
233242

0 commit comments

Comments
 (0)