From 37ca236d095d039b48899720ff39a630f4f0dff2 Mon Sep 17 00:00:00 2001 From: Charlie Croom Date: Sat, 11 May 2019 10:05:35 -0700 Subject: [PATCH] [fix] Image displays defaultSource until source load completes This fixes an issue that would cause the defaultSource to be removed as soon as the source beings to load. The original intent was to support progressive JPEGs. However, in cases where a defaultSource has been provided, we should respect the intent to display it until the primary source is ready to immediately replace the defaultSource. Close #1345 --- .../src/exports/Image/__tests__/index-test.js | 29 ++++++++++++------- .../src/exports/Image/index.js | 16 ++++++---- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/packages/react-native-web/src/exports/Image/__tests__/index-test.js b/packages/react-native-web/src/exports/Image/__tests__/index-test.js index 5aa8d334..b9dfa9ae 100644 --- a/packages/react-native-web/src/exports/Image/__tests__/index-test.js +++ b/packages/react-native-web/src/exports/Image/__tests__/index-test.js @@ -5,8 +5,8 @@ import Image from '../'; import ImageLoader from '../../../modules/ImageLoader'; import ImageUriCache from '../ImageUriCache'; import React from 'react'; +import { shallow } from 'enzyme'; import StyleSheet from '../../StyleSheet'; -import { mount, shallow } from 'enzyme'; const originalImage = window.Image; @@ -121,22 +121,18 @@ describe('components/Image', () => { }); test('is called on update if "uri" is different', () => { - jest.useFakeTimers(); const onLoadStub = jest.fn(); const uri = 'https://test.com/img.jpg'; - const component = mount(); + const component = shallow(); component.setProps({ source: 'https://blah.com/img.png' }); - jest.runOnlyPendingTimers(); expect(onLoadStub.mock.calls.length).toBe(2); }); test('is not called on update if "uri" is the same', () => { - jest.useFakeTimers(); const onLoadStub = jest.fn(); const uri = 'https://test.com/img.jpg'; - const component = mount(); + const component = shallow(); component.setProps({ resizeMode: 'stretch' }); - jest.runOnlyPendingTimers(); expect(onLoadStub.mock.calls.length).toBe(1); }); }); @@ -185,7 +181,7 @@ describe('components/Image', () => { ImageUriCache.add(uriTwo); // initial render - const component = mount(); + const component = shallow(); ImageUriCache.remove(uriOne); expect( component @@ -206,11 +202,9 @@ describe('components/Image', () => { }); test('is correctly updated when missing in initial render', () => { - jest.useFakeTimers(); const uri = 'https://testing.com/img.jpg'; - const component = mount(); + const component = shallow(); component.setProps({ source: { uri } }); - jest.runOnlyPendingTimers(); expect( component .render() @@ -218,6 +212,19 @@ describe('components/Image', () => { .attr('src') ).toBe(uri); }); + + test('is correctly updated only when loaded if defaultSource provided', () => { + const defaultUri = 'https://testing.com/preview.jpg'; + const uri = 'https://testing.com/fullSize.jpg'; + let loadCallback; + ImageLoader.load = jest.fn().mockImplementationOnce((_, onLoad, onError) => { + loadCallback = onLoad; + }); + const component = shallow(); + expect(component.find('img').prop('src')).toBe(defaultUri); + loadCallback(); + expect(component.find('img').prop('src')).toBe(uri); + }); }); describe('prop "style"', () => { diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index 5695ecd9..2262a68e 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -177,11 +177,14 @@ class Image extends Component<*, State> { componentDidUpdate(prevProps) { const prevUri = resolveAssetUri(prevProps.source); const uri = resolveAssetUri(this.props.source); + const hasDefaultSource = this.props.defaultSource != null; if (prevUri !== uri) { ImageUriCache.remove(prevUri); const isPreviouslyLoaded = ImageUriCache.has(uri); isPreviouslyLoaded && ImageUriCache.add(uri); - this._updateImageState(getImageState(uri, isPreviouslyLoaded)); + this._updateImageState(getImageState(uri, isPreviouslyLoaded), hasDefaultSource); + } else if (hasDefaultSource && prevProps.defaultSource !== this.props.defaultSource) { + this._updateImageState(this._imageState, hasDefaultSource); } if (this._imageState === STATUS_PENDING) { this._createImageLoader(); @@ -379,8 +382,8 @@ class Image extends Component<*, State> { } _onLoadStart() { - const { onLoadStart } = this.props; - this._updateImageState(STATUS_LOADING); + const { defaultSource, onLoadStart } = this.props; + this._updateImageState(STATUS_LOADING, defaultSource != null); if (onLoadStart) { onLoadStart(); } @@ -390,11 +393,12 @@ class Image extends Component<*, State> { this._imageRef = ref; }; - _updateImageState(status) { + _updateImageState(status: ?string, hasDefaultSource: ?boolean = false) { this._imageState = status; const shouldDisplaySource = - this._imageState === STATUS_LOADED || this._imageState === STATUS_LOADING; - // only triggers a re-render when the image is loading (to support PJEG), loaded, or failed + this._imageState === STATUS_LOADED || + (this._imageState === STATUS_LOADING && !hasDefaultSource); + // only triggers a re-render when the image is loading and has no default image (to support PJPEG), loaded, or failed if (shouldDisplaySource !== this.state.shouldDisplaySource) { if (this._isMounted) { this.setState(() => ({ shouldDisplaySource }));