Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RNMobile] Update Image component to avoid flicker when updating the URI #57869

Merged
merged 20 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
5e43b3e
Update native image component to avoid flicker when updating the URI
derekblank Jan 16, 2024
9d0797a
Update non-visible Image block styles
derekblank Jan 17, 2024
af417cf
Update mobile Image block to use separate network and local image URL…
derekblank Jan 17, 2024
f7c9232
Update Image block platform fetching logic
derekblank Jan 17, 2024
c2d436b
Update Android logic for Image block platform fetching logic
derekblank Jan 17, 2024
7c032ce
Add further updates to Image block platform logic
derekblank Jan 18, 2024
bd29600
Fix code formatting issue in Image block source prop
derekblank Jan 18, 2024
cd7b8e2
Resolve RNImage.prefetch promise
derekblank Jan 18, 2024
efe77ae
Update CHANGELOG
derekblank Jan 19, 2024
607af40
Update non-visible image styles
derekblank Jan 22, 2024
c2ef868
Remove duplicate references to localURL
derekblank Jan 24, 2024
0b3839a
Fix Replace Image behavior to update from localURL
derekblank Jan 24, 2024
e71e4bf
Refactor Image block URL handling logic
derekblank Jan 24, 2024
553c243
test: Fix failures due to image component modifications (#58213)
dcalhoun Jan 24, 2024
70b003a
Update nonVisibleImage styles
derekblank Jan 25, 2024
4abc0c5
Merge branch 'rnmobile/prevent-image-flicker-when-uploading' of https…
derekblank Jan 25, 2024
541a242
Update packages/components/src/mobile/image/index.native.js
derekblank Jan 30, 2024
ee04f79
Update RNImage error handler event with code comment
derekblank Jan 30, 2024
5c4d2d7
Merge branch 'rnmobile/prevent-image-flicker-when-uploading' of https…
derekblank Jan 30, 2024
9718e9d
Update selected images styles on iOS to account for non-visible image
derekblank Jan 31, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 100 additions & 11 deletions packages/components/src/mobile/image/index.native.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { Image as RNImage, Text, View } from 'react-native';
import { Animated, Image as RNImage, Text, View } from 'react-native';
import FastImage from 'react-native-fast-image';

/**
Expand All @@ -11,7 +11,7 @@ import { __ } from '@wordpress/i18n';
import { Icon } from '@wordpress/components';
import { image, offline } from '@wordpress/icons';
import { usePreferredColorSchemeStyle } from '@wordpress/compose';
import { useEffect, useState, Platform } from '@wordpress/element';
import { useEffect, useState, useRef, Platform } from '@wordpress/element';

/**
* Internal dependencies
Expand Down Expand Up @@ -54,6 +54,9 @@ const ImageComponent = ( {
} ) => {
const [ imageData, setImageData ] = useState( null );
const [ containerSize, setContainerSize ] = useState( null );
const [ localURL, setLocalURL ] = useState( null );
const [ networkURL, setNetworkURL ] = useState( null );
const [ networkImageLoaded, setNetworkImageLoaded ] = useState( false );

// Disabled for Android due to https://github.com/WordPress/gutenberg/issues/43149
const Image =
Expand All @@ -80,6 +83,33 @@ const ImageComponent = ( {
onImageDataLoad( metaData );
}
} );

if ( url.startsWith( 'file:///' ) ) {
setLocalURL( url );
derekblank marked this conversation as resolved.
Show resolved Hide resolved
setNetworkURL( null );
setNetworkImageLoaded( false );
} else if ( url.startsWith( 'https://' ) ) {
if ( Platform.isIOS ) {
setNetworkURL( url );
} else if ( Platform.isAndroid ) {
RNImage.prefetch( url ).then(
() => {
if ( ! isCurrent ) {
return;
}
setNetworkURL( url );
setNetworkImageLoaded( true );
dcalhoun marked this conversation as resolved.
Show resolved Hide resolved
},
() => {
// This callback is called when the image fails to load,
// but these events are handled by `isUploadFailed`
// and `isUploadPaused` events instead.
//
// Ignoring the error event will persist the local image URI.
}
);
}
}
}
return () => ( isCurrent = false );
// Disable reason: deferring this refactor to the native team.
Expand Down Expand Up @@ -188,9 +218,19 @@ const ImageComponent = ( {
focalPoint && styles.focalPointContainer,
];

const opacityValue = useRef( new Animated.Value( 1 ) ).current;

useEffect( () => {
Animated.timing( opacityValue, {
toValue: isUploadInProgress ? 0.3 : 1,
duration: 100,
useNativeDriver: true,
} ).start();
}, [ isUploadInProgress, opacityValue ] );

const imageStyles = [
{
opacity: isUploadInProgress ? 0.3 : 1,
opacity: opacityValue,
height: containerSize?.height,
},
! resizeMode && {
Expand All @@ -214,6 +254,7 @@ const ImageComponent = ( {
imageHeight && { height: imageHeight },
shapeStyle,
];

const imageSelectedStyles = [
usePreferredColorSchemeStyle(
styles.imageBorder,
Expand Down Expand Up @@ -259,14 +300,62 @@ const ImageComponent = ( {
</View>
) : (
<View style={ focalPoint && styles.focalPointContent }>
<Image
style={ imageStyles }
source={ { uri: url } }
{ ...( ! focalPoint && {
resizeMethod: 'scale',
} ) }
resizeMode={ imageResizeMode }
/>
{ Platform.isAndroid && (
<>
{ networkImageLoaded && networkURL && (
<Animated.Image
style={ imageStyles }
fadeDuration={ 0 }
source={ { uri: networkURL } }
{ ...( ! focalPoint && {
resizeMethod: 'scale',
} ) }
resizeMode={ imageResizeMode }
testID={ `network-image-${ url }` }
/>
) }
{ ! networkImageLoaded && ! networkURL && (
<Animated.Image
style={ imageStyles }
fadeDuration={ 0 }
source={ { uri: localURL } }
{ ...( ! focalPoint && {
resizeMethod: 'scale',
} ) }
resizeMode={ imageResizeMode }
/>
) }
</>
) }
{ Platform.isIOS && (
<>
<Animated.Image
style={ imageStyles }
source={ {
uri:
networkURL && networkImageLoaded
? networkURL
: localURL || url,
} }
{ ...( ! focalPoint && {
resizeMethod: 'scale',
} ) }
resizeMode={ imageResizeMode }
testID={ `network-image-${
networkURL && networkImageLoaded
? networkURL
: localURL || url
}` }
/>
<Image
source={ { uri: networkURL } }
style={ styles.nonVisibleImage }
onLoad={ () => {
setNetworkImageLoaded( true );
dcalhoun marked this conversation as resolved.
Show resolved Hide resolved
} }
/>
</>
) }
</View>
) }

Expand Down
6 changes: 6 additions & 0 deletions packages/components/src/mobile/image/style.native.scss
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,9 @@
.wide {
width: 100%;
}

.nonVisibleImage {
height: 1;
width: 1;
opacity: 0;
}
13 changes: 8 additions & 5 deletions packages/edit-post/src/test/editor.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,10 @@ describe( 'Editor', () => {
await initializeEditor();

// Act
await act( () => mediaAppendCallback( MEDIA[ 0 ] ) );
await act( () => mediaAppendCallback( MEDIA[ 2 ] ) );
act( () => mediaAppendCallback( MEDIA[ 0 ] ) );
act( () => mediaAppendCallback( MEDIA[ 2 ] ) );
await screen.findByTestId( `network-image-${ MEDIA[ 0 ].serverUrl }` );
await screen.findByTestId( `network-image-${ MEDIA[ 2 ].serverUrl }` );

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

// Act
await act( () => mediaAppendCallback( MEDIA[ 0 ] ) );
act( () => mediaAppendCallback( MEDIA[ 0 ] ) );
// Unsupported type (PDF file)
await act( () => mediaAppendCallback( MEDIA[ 1 ] ) );
await act( () => mediaAppendCallback( MEDIA[ 3 ] ) );
act( () => mediaAppendCallback( MEDIA[ 1 ] ) );
act( () => mediaAppendCallback( MEDIA[ 3 ] ) );
await screen.findByTestId( `network-image-${ MEDIA[ 0 ].serverUrl }` );

// Assert
expect( getEditorHtml() ).toMatchSnapshot();
Expand Down
1 change: 1 addition & 0 deletions packages/react-native-editor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ For each user feature we should also add a importance categorization label to i
- [**] Image block media uploads display a custom error message when there is no internet connection [#56937]
- [*] Fix missing custom color indicator for custom gradients [#57605]
- [**] Display a notice when a network connection unavailable [#56934]
- [**] Prevent images from temporarily disappearing when uploading media [#57869]

## 1.110.0
- [*] [internal] Move InserterButton from components package to block-editor package [#56494]
Expand Down
8 changes: 8 additions & 0 deletions test/native/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,14 @@ jest.spyOn( Image, 'getSize' ).mockImplementation( ( url, success ) =>
success( 0, 0 )
);

jest.spyOn( Image, 'prefetch' ).mockImplementation(
( url, callback = () => {} ) => {
const mockRequestId = `mockRequestId-${ url }`;
callback( mockRequestId );
return Promise.resolve( true );
}
);

jest.mock( 'react-native/Libraries/Utilities/BackHandler', () => {
return jest.requireActual(
'react-native/Libraries/Utilities/__mocks__/BackHandler.js'
Expand Down
Loading