[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
This commit is contained in:
Charlie Croom
2019-05-11 10:05:35 -07:00
committed by Nicolas Gallagher
parent 950bfd039c
commit 37ca236d09
2 changed files with 28 additions and 17 deletions
@@ -5,8 +5,8 @@ import Image from '../';
import ImageLoader from '../../../modules/ImageLoader'; import ImageLoader from '../../../modules/ImageLoader';
import ImageUriCache from '../ImageUriCache'; import ImageUriCache from '../ImageUriCache';
import React from 'react'; import React from 'react';
import { shallow } from 'enzyme';
import StyleSheet from '../../StyleSheet'; import StyleSheet from '../../StyleSheet';
import { mount, shallow } from 'enzyme';
const originalImage = window.Image; const originalImage = window.Image;
@@ -121,22 +121,18 @@ describe('components/Image', () => {
}); });
test('is called on update if "uri" is different', () => { test('is called on update if "uri" is different', () => {
jest.useFakeTimers();
const onLoadStub = jest.fn(); const onLoadStub = jest.fn();
const uri = 'https://test.com/img.jpg'; const uri = 'https://test.com/img.jpg';
const component = mount(<Image onLoad={onLoadStub} source={uri} />); const component = shallow(<Image onLoad={onLoadStub} source={uri} />);
component.setProps({ source: 'https://blah.com/img.png' }); component.setProps({ source: 'https://blah.com/img.png' });
jest.runOnlyPendingTimers();
expect(onLoadStub.mock.calls.length).toBe(2); expect(onLoadStub.mock.calls.length).toBe(2);
}); });
test('is not called on update if "uri" is the same', () => { test('is not called on update if "uri" is the same', () => {
jest.useFakeTimers();
const onLoadStub = jest.fn(); const onLoadStub = jest.fn();
const uri = 'https://test.com/img.jpg'; const uri = 'https://test.com/img.jpg';
const component = mount(<Image onLoad={onLoadStub} source={uri} />); const component = shallow(<Image onLoad={onLoadStub} source={uri} />);
component.setProps({ resizeMode: 'stretch' }); component.setProps({ resizeMode: 'stretch' });
jest.runOnlyPendingTimers();
expect(onLoadStub.mock.calls.length).toBe(1); expect(onLoadStub.mock.calls.length).toBe(1);
}); });
}); });
@@ -185,7 +181,7 @@ describe('components/Image', () => {
ImageUriCache.add(uriTwo); ImageUriCache.add(uriTwo);
// initial render // initial render
const component = mount(<Image source={{ uri: uriOne }} />); const component = shallow(<Image source={{ uri: uriOne }} />);
ImageUriCache.remove(uriOne); ImageUriCache.remove(uriOne);
expect( expect(
component component
@@ -206,11 +202,9 @@ describe('components/Image', () => {
}); });
test('is correctly updated when missing in initial render', () => { test('is correctly updated when missing in initial render', () => {
jest.useFakeTimers();
const uri = 'https://testing.com/img.jpg'; const uri = 'https://testing.com/img.jpg';
const component = mount(<Image />); const component = shallow(<Image />);
component.setProps({ source: { uri } }); component.setProps({ source: { uri } });
jest.runOnlyPendingTimers();
expect( expect(
component component
.render() .render()
@@ -218,6 +212,19 @@ describe('components/Image', () => {
.attr('src') .attr('src')
).toBe(uri); ).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(<Image defaultSource={{ uri: defaultUri }} source={{ uri }} />);
expect(component.find('img').prop('src')).toBe(defaultUri);
loadCallback();
expect(component.find('img').prop('src')).toBe(uri);
});
}); });
describe('prop "style"', () => { describe('prop "style"', () => {
+10 -6
View File
@@ -177,11 +177,14 @@ class Image extends Component<*, State> {
componentDidUpdate(prevProps) { componentDidUpdate(prevProps) {
const prevUri = resolveAssetUri(prevProps.source); const prevUri = resolveAssetUri(prevProps.source);
const uri = resolveAssetUri(this.props.source); const uri = resolveAssetUri(this.props.source);
const hasDefaultSource = this.props.defaultSource != null;
if (prevUri !== uri) { if (prevUri !== uri) {
ImageUriCache.remove(prevUri); ImageUriCache.remove(prevUri);
const isPreviouslyLoaded = ImageUriCache.has(uri); const isPreviouslyLoaded = ImageUriCache.has(uri);
isPreviouslyLoaded && ImageUriCache.add(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) { if (this._imageState === STATUS_PENDING) {
this._createImageLoader(); this._createImageLoader();
@@ -379,8 +382,8 @@ class Image extends Component<*, State> {
} }
_onLoadStart() { _onLoadStart() {
const { onLoadStart } = this.props; const { defaultSource, onLoadStart } = this.props;
this._updateImageState(STATUS_LOADING); this._updateImageState(STATUS_LOADING, defaultSource != null);
if (onLoadStart) { if (onLoadStart) {
onLoadStart(); onLoadStart();
} }
@@ -390,11 +393,12 @@ class Image extends Component<*, State> {
this._imageRef = ref; this._imageRef = ref;
}; };
_updateImageState(status) { _updateImageState(status: ?string, hasDefaultSource: ?boolean = false) {
this._imageState = status; this._imageState = status;
const shouldDisplaySource = const shouldDisplaySource =
this._imageState === STATUS_LOADED || this._imageState === STATUS_LOADING; this._imageState === STATUS_LOADED ||
// only triggers a re-render when the image is loading (to support PJEG), loaded, or failed (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 (shouldDisplaySource !== this.state.shouldDisplaySource) {
if (this._isMounted) { if (this._isMounted) {
this.setState(() => ({ shouldDisplaySource })); this.setState(() => ({ shouldDisplaySource }));