Skip to content

Commit 7b77931

Browse files
derekblankdcalhoun
andauthored
[RNMobile] Update Image component to avoid flicker when updating the URI (#57869)
* Update native image component to avoid flicker when updating the URI * Update non-visible Image block styles * Update mobile Image block to use separate network and local image URL behavior between platforms * Update Image block platform fetching logic * Update Android logic for Image block platform fetching logic * Add further updates to Image block platform logic * Fix code formatting issue in Image block source prop * Resolve RNImage.prefetch promise * Update CHANGELOG * Update non-visible image styles Resolves layout display issue when block is selected * Remove duplicate references to localURL * Fix Replace Image behavior to update from localURL * Refactor Image block URL handling logic * test: Fix failures due to image component modifications (#58213) The mobile image component introduce new asynchronous effects. We must await the result of those effect, in the form of asserting UI changes from the subsequent state updates, to satisfy the React test utilities expectation that unexpected re-renders do not occur after the test completes. Additionally, there were instances of awaiting `act` invocations that were not asynchronous. The erroneous usage of `await` for these synchronous `act` calls resulted in cryptic errors. In reality, the logic run within these `act` calls are synchronous and should merely be wrapped in `act` to signal that they result in a re-render. * Update nonVisibleImage styles Using width: 1 and height: 1 ensures that onLoad will fire * Update packages/components/src/mobile/image/index.native.js Co-authored-by: David Calhoun <[email protected]> * Update RNImage error handler event with code comment * Update selected images styles on iOS to account for non-visible image On iOS, add 1 to height to account for the 1px non-visible image that is used to determine when the network image has loaded We also must verify that it is not NaN, as it can be NaN when the image is loading. --------- Co-authored-by: David Calhoun <[email protected]>
1 parent ceb3c1c commit 7b77931

File tree

5 files changed

+140
-17
lines changed

5 files changed

+140
-17
lines changed

packages/components/src/mobile/image/index.native.js

Lines changed: 117 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* External dependencies
33
*/
4-
import { Image as RNImage, Text, View } from 'react-native';
4+
import { Animated, Image as RNImage, Text, View } from 'react-native';
55
import FastImage from 'react-native-fast-image';
66

77
/**
@@ -11,7 +11,7 @@ import { __ } from '@wordpress/i18n';
1111
import { Icon } from '@wordpress/components';
1212
import { image, offline } from '@wordpress/icons';
1313
import { usePreferredColorSchemeStyle } from '@wordpress/compose';
14-
import { useEffect, useState, Platform } from '@wordpress/element';
14+
import { useEffect, useState, useRef, Platform } from '@wordpress/element';
1515

1616
/**
1717
* Internal dependencies
@@ -54,6 +54,9 @@ const ImageComponent = ( {
5454
} ) => {
5555
const [ imageData, setImageData ] = useState( null );
5656
const [ containerSize, setContainerSize ] = useState( null );
57+
const [ localURL, setLocalURL ] = useState( null );
58+
const [ networkURL, setNetworkURL ] = useState( null );
59+
const [ networkImageLoaded, setNetworkImageLoaded ] = useState( false );
5760

5861
// Disabled for Android due to https://github.com/WordPress/gutenberg/issues/43149
5962
const Image =
@@ -80,6 +83,33 @@ const ImageComponent = ( {
8083
onImageDataLoad( metaData );
8184
}
8285
} );
86+
87+
if ( url.startsWith( 'file:///' ) ) {
88+
setLocalURL( url );
89+
setNetworkURL( null );
90+
setNetworkImageLoaded( false );
91+
} else if ( url.startsWith( 'https://' ) ) {
92+
if ( Platform.isIOS ) {
93+
setNetworkURL( url );
94+
} else if ( Platform.isAndroid ) {
95+
RNImage.prefetch( url ).then(
96+
() => {
97+
if ( ! isCurrent ) {
98+
return;
99+
}
100+
setNetworkURL( url );
101+
setNetworkImageLoaded( true );
102+
},
103+
() => {
104+
// This callback is called when the image fails to load,
105+
// but these events are handled by `isUploadFailed`
106+
// and `isUploadPaused` events instead.
107+
//
108+
// Ignoring the error event will persist the local image URI.
109+
}
110+
);
111+
}
112+
}
83113
}
84114
return () => ( isCurrent = false );
85115
// Disable reason: deferring this refactor to the native team.
@@ -188,9 +218,19 @@ const ImageComponent = ( {
188218
focalPoint && styles.focalPointContainer,
189219
];
190220

221+
const opacityValue = useRef( new Animated.Value( 1 ) ).current;
222+
223+
useEffect( () => {
224+
Animated.timing( opacityValue, {
225+
toValue: isUploadInProgress ? 0.3 : 1,
226+
duration: 100,
227+
useNativeDriver: true,
228+
} ).start();
229+
}, [ isUploadInProgress, opacityValue ] );
230+
191231
const imageStyles = [
192232
{
193-
opacity: isUploadInProgress ? 0.3 : 1,
233+
opacity: opacityValue,
194234
height: containerSize?.height,
195235
},
196236
! resizeMode && {
@@ -214,12 +254,29 @@ const ImageComponent = ( {
214254
imageHeight && { height: imageHeight },
215255
shapeStyle,
216256
];
257+
258+
// On iOS, add 1 to height to account for the 1px non-visible image
259+
// that is used to determine when the network image has loaded
260+
// We also must verify that it is not NaN, as it can be NaN when the image is loading.
261+
// This is not necessary on Android as the non-visible image is not used.
262+
let calculatedSelectedHeight;
263+
if ( Platform.isIOS ) {
264+
calculatedSelectedHeight =
265+
containerSize && ! isNaN( containerSize.height )
266+
? containerSize.height + 1
267+
: 0;
268+
} else {
269+
calculatedSelectedHeight = containerSize?.height;
270+
}
271+
217272
const imageSelectedStyles = [
218273
usePreferredColorSchemeStyle(
219274
styles.imageBorder,
220275
styles.imageBorderDark
221276
),
222-
{ height: containerSize?.height },
277+
{
278+
height: calculatedSelectedHeight,
279+
},
223280
];
224281

225282
return (
@@ -259,14 +316,62 @@ const ImageComponent = ( {
259316
</View>
260317
) : (
261318
<View style={ focalPoint && styles.focalPointContent }>
262-
<Image
263-
style={ imageStyles }
264-
source={ { uri: url } }
265-
{ ...( ! focalPoint && {
266-
resizeMethod: 'scale',
267-
} ) }
268-
resizeMode={ imageResizeMode }
269-
/>
319+
{ Platform.isAndroid && (
320+
<>
321+
{ networkImageLoaded && networkURL && (
322+
<Animated.Image
323+
style={ imageStyles }
324+
fadeDuration={ 0 }
325+
source={ { uri: networkURL } }
326+
{ ...( ! focalPoint && {
327+
resizeMethod: 'scale',
328+
} ) }
329+
resizeMode={ imageResizeMode }
330+
testID={ `network-image-${ url }` }
331+
/>
332+
) }
333+
{ ! networkImageLoaded && ! networkURL && (
334+
<Animated.Image
335+
style={ imageStyles }
336+
fadeDuration={ 0 }
337+
source={ { uri: localURL } }
338+
{ ...( ! focalPoint && {
339+
resizeMethod: 'scale',
340+
} ) }
341+
resizeMode={ imageResizeMode }
342+
/>
343+
) }
344+
</>
345+
) }
346+
{ Platform.isIOS && (
347+
<>
348+
<Animated.Image
349+
style={ imageStyles }
350+
source={ {
351+
uri:
352+
networkURL && networkImageLoaded
353+
? networkURL
354+
: localURL || url,
355+
} }
356+
{ ...( ! focalPoint && {
357+
resizeMethod: 'scale',
358+
} ) }
359+
resizeMode={ imageResizeMode }
360+
testID={ `network-image-${
361+
networkURL && networkImageLoaded
362+
? networkURL
363+
: localURL || url
364+
}` }
365+
/>
366+
<Image
367+
source={ { uri: networkURL } }
368+
style={ styles.nonVisibleImage }
369+
onLoad={ () => {
370+
setNetworkImageLoaded( true );
371+
} }
372+
/>
373+
</>
374+
) }
270375
</View>
271376
) }
272377

packages/components/src/mobile/image/style.native.scss

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,3 +171,9 @@
171171
.wide {
172172
width: 100%;
173173
}
174+
175+
.nonVisibleImage {
176+
height: 1;
177+
width: 1;
178+
opacity: 0;
179+
}

packages/edit-post/src/test/editor.native.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,10 @@ describe( 'Editor', () => {
103103
await initializeEditor();
104104

105105
// Act
106-
await act( () => mediaAppendCallback( MEDIA[ 0 ] ) );
107-
await act( () => mediaAppendCallback( MEDIA[ 2 ] ) );
106+
act( () => mediaAppendCallback( MEDIA[ 0 ] ) );
107+
act( () => mediaAppendCallback( MEDIA[ 2 ] ) );
108+
await screen.findByTestId( `network-image-${ MEDIA[ 0 ].serverUrl }` );
109+
await screen.findByTestId( `network-image-${ MEDIA[ 2 ].serverUrl }` );
108110

109111
// Assert
110112
expect( getEditorHtml() ).toMatchSnapshot();
@@ -122,10 +124,11 @@ describe( 'Editor', () => {
122124
await initializeEditor();
123125

124126
// Act
125-
await act( () => mediaAppendCallback( MEDIA[ 0 ] ) );
127+
act( () => mediaAppendCallback( MEDIA[ 0 ] ) );
126128
// Unsupported type (PDF file)
127-
await act( () => mediaAppendCallback( MEDIA[ 1 ] ) );
128-
await act( () => mediaAppendCallback( MEDIA[ 3 ] ) );
129+
act( () => mediaAppendCallback( MEDIA[ 1 ] ) );
130+
act( () => mediaAppendCallback( MEDIA[ 3 ] ) );
131+
await screen.findByTestId( `network-image-${ MEDIA[ 0 ].serverUrl }` );
129132

130133
// Assert
131134
expect( getEditorHtml() ).toMatchSnapshot();

packages/react-native-editor/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ For each user feature we should also add a importance categorization label to i
2323
- [**] Image block media uploads display a custom error message when there is no internet connection [#56937]
2424
- [*] Fix missing custom color indicator for custom gradients [#57605]
2525
- [**] Display a notice when a network connection unavailable [#56934]
26+
- [**] Prevent images from temporarily disappearing when uploading media [#57869]
2627

2728
## 1.110.1
2829
- [**] Fix crash when RichText values are not defined [#58088]

test/native/setup.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,14 @@ jest.spyOn( Image, 'getSize' ).mockImplementation( ( url, success ) =>
284284
success( 0, 0 )
285285
);
286286

287+
jest.spyOn( Image, 'prefetch' ).mockImplementation(
288+
( url, callback = () => {} ) => {
289+
const mockRequestId = `mockRequestId-${ url }`;
290+
callback( mockRequestId );
291+
return Promise.resolve( true );
292+
}
293+
);
294+
287295
jest.mock( 'react-native/Libraries/Utilities/BackHandler', () => {
288296
return jest.requireActual(
289297
'react-native/Libraries/Utilities/__mocks__/BackHandler.js'

0 commit comments

Comments
 (0)