Skip to content

Commit

Permalink
[fix] Image: image LOADED state is only captured initially
Browse files Browse the repository at this point in the history
If the Image component is rendered with a `null` source, and consecutively
updated with actual source url that was already loaded, it would fail to
pick up the change - `state` would be `IDLE` for a brief moment and
this would cause a small flicker when the image renders

Let's always start from IDLE state, and update `shouldDisplaySource`
condition to be based on `ImageLoader.has` cache or not

Fix #2492
  • Loading branch information
kidroca authored and necolas committed Apr 10, 2023
1 parent 160f50f commit c47935c
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ exports[`components/Image prop "source" is correctly updated when missing in ini
</div>
`;

exports[`components/Image prop "source" is not set immediately if the image has not already been loaded 1`] = `
exports[`components/Image prop "source" is set immediately if the image has already been loaded 1`] = `
<div
class="css-view-175oi2r r-flexBasis-1mlwlqe r-overflow-1udh08x r-zIndex-417010"
>
Expand All @@ -272,53 +272,53 @@ exports[`components/Image prop "source" is not set immediately if the image has
</div>
`;

exports[`components/Image prop "source" is set immediately if the image has already been loaded 1`] = `
exports[`components/Image prop "source" is set immediately if the image has already been loaded 2`] = `
<div
class="css-view-175oi2r r-flexBasis-1mlwlqe r-overflow-1udh08x r-zIndex-417010"
>
<div
class="css-view-175oi2r r-backgroundColor-1niwhzg r-backgroundPosition-vvn4in r-backgroundRepeat-u6sd8q r-bottom-1p0dtai r-height-1pi2tsx r-left-1d2f490 r-position-u8s1d r-right-zchlnj r-top-ipm5af r-width-13qz1uu r-zIndex-1wyyakw r-backgroundSize-4gszlv"
style="background-image: url(https://google.com/favicon.ico);"
style="background-image: url(https://twitter.com/favicon.ico);"
/>
<img
alt=""
class="css-accessibilityImage-9pa8cd"
draggable="false"
src="https://google.com/favicon.ico"
src="https://twitter.com/favicon.ico"
/>
</div>
`;

exports[`components/Image prop "source" is set immediately if the image has already been loaded 2`] = `
exports[`components/Image prop "source" is set immediately if the image was preloaded 1`] = `
<div
class="css-view-175oi2r r-flexBasis-1mlwlqe r-overflow-1udh08x r-zIndex-417010"
>
<div
class="css-view-175oi2r r-backgroundColor-1niwhzg r-backgroundPosition-vvn4in r-backgroundRepeat-u6sd8q r-bottom-1p0dtai r-height-1pi2tsx r-left-1d2f490 r-position-u8s1d r-right-zchlnj r-top-ipm5af r-width-13qz1uu r-zIndex-1wyyakw r-backgroundSize-4gszlv"
style="background-image: url(https://twitter.com/favicon.ico);"
style="background-image: url(https://yahoo.com/favicon.ico);"
/>
<img
alt=""
class="css-accessibilityImage-9pa8cd"
draggable="false"
src="https://twitter.com/favicon.ico"
src="https://yahoo.com/favicon.ico"
/>
</div>
`;

exports[`components/Image prop "source" is set immediately if the image was preloaded 1`] = `
exports[`components/Image prop "source" is set immediately while image is loading and there is no default source 1`] = `
<div
class="css-view-175oi2r r-flexBasis-1mlwlqe r-overflow-1udh08x r-zIndex-417010"
>
<div
class="css-view-175oi2r r-backgroundColor-1niwhzg r-backgroundPosition-vvn4in r-backgroundRepeat-u6sd8q r-bottom-1p0dtai r-height-1pi2tsx r-left-1d2f490 r-position-u8s1d r-right-zchlnj r-top-ipm5af r-width-13qz1uu r-zIndex-1wyyakw r-backgroundSize-4gszlv"
style="background-image: url(https://yahoo.com/favicon.ico);"
style="background-image: url(https://google.com/not-yet-loaded-image.ico);"
/>
<img
alt=""
class="css-accessibilityImage-9pa8cd"
draggable="false"
src="https://yahoo.com/favicon.ico"
src="https://google.com/not-yet-loaded-image.ico"
/>
</div>
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,8 @@ describe('components/Image', () => {
});
});

test('is not set immediately if the image has not already been loaded', () => {
const uri = 'https://google.com/favicon.ico';
test('is set immediately while image is loading and there is no default source', () => {
const uri = 'https://google.com/not-yet-loaded-image.ico';
const source = { uri };
const { container } = render(<Image source={source} />);
expect(container.firstChild).toMatchSnapshot();
Expand Down
19 changes: 6 additions & 13 deletions packages/react-native-web/src/exports/Image/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,24 +225,18 @@ const BaseImage: ImageComponent = React.forwardRef((props, ref) => {
}
}

const [state, updateState] = React.useState(() => {
const uri = resolveAssetUri(source);
if (uri != null) {
const isLoaded = ImageLoader.has(uri);
if (isLoaded) {
return LOADED;
}
}
return IDLE;
});

const [state, updateState] = React.useState(IDLE);
const [layout, updateLayout] = React.useState({});
const hasTextAncestor = React.useContext(TextAncestorContext);
const hiddenImageRef = React.useRef(null);
const filterRef = React.useRef(_filterId++);
const requestRef = React.useRef(null);
const uri = resolveAssetUri(source);
const isCached = uri != null && ImageLoader.has(uri);
const shouldDisplaySource =
state === LOADED || (state === LOADING && defaultSource == null);
state === LOADED ||
isCached ||
(state === LOADING && defaultSource == null);
const [flatStyle, _resizeMode, filter, _tintColor] = getFlatStyle(
style,
blurRadius,
Expand Down Expand Up @@ -297,7 +291,6 @@ const BaseImage: ImageComponent = React.forwardRef((props, ref) => {
}

// Image loading
const uri = resolveAssetUri(source);
React.useEffect(() => {
abortPendingRequest();

Expand Down

0 comments on commit c47935c

Please sign in to comment.