Apply skeleton for message images and on workspace avatar

[diegolmello]

Overview

We’ve been rendering a loading indicator for images, but we introduced the skeleton usage for videoconf elements and on the web.

It’s a better experience if we unify this behavior.

To be done

• Apply skeleton on WorkspaceView’s ServerAvatar
• Apply skeleton on Message’s MessageImage
• Apply skeleton on Markdown’s MessageImage
• Remove react-native-image-progress and react-native-progress

[Misterr-H]

@diegolmello hii i want to work on this

[dudanogueira]

@diegolmello hii i want to work on this

Awesome! Let us know if you need assistance!

[Misterr-H]

@diegolmello can you please reference me the videoconf elements where there is the usage of skeleton?

[diegolmello]

Hey. Here it is https://github.com/RocketChat/Rocket.Chat.ReactNative/blob/develop/app/containers/UIKit/VideoConferenceBlock/components/VideoConferenceSkeletonLoading.tsx

[Misterr-H]

Hii @diegolmello got both the skeleton component and react-native-image-progress component thus in order to replace the usage of react-native-image-progress in WorkspaceView’s ServerAvatar i'm creating a skeleton component and if image loading is success return image otherwise skeleton something like imageLoad.success ? return Image : return SkeletonComponent

[diegolmello]

@Misterr-H You don't need that. Image contains onLoadStart and onLoadEnd.

[Misterr-H]

@diegolmello where can i use useState hook in this functional component to be used in onLoadStart and onLoadEnd

[Misterr-H]

@diegolmello Please check this code for ServerAvatar. This is working fine. The only problem is Skeleton appears on the Top of the actual image position. I'm unable to figure out how to place it on the actual image position.

[floels]

Hi @diegolmello and @Misterr-H,

I am willing to help implement this improvement.

Here is the approach I have in mind:

1. Create a new <FastImageWithSkeletonPlaceholder /> component, based on both <FastImage /> and <SkeletonPlaceholder />, which displays a skeleton placeholder until the <FastImage /> finished loading. Here is some pseudo-code to illustrate how it would work:

export const FastImageWithSkeletonPlaceholder = (props) => {
    const [isLoading, setLoading] = useState(true);

    const handleLoadEnd = () => {
         setLoading(false);
    }

    return (
        <View>
            {isLoading && placeholder}
            <FastImage
                style={[{ display: isLoading ? 'none' : 'flex' }, ...props.style]}
                onLoadEnd={handleLoadEnd}
                {...otherProps}
            />
        </View>
    );
};

<FastImageWithSkeletonPlaceholder /> would pass some of its props down to <FastImage />, to make sure <FastImage /> can be customized as needed in the various use cases listed. The exact design of the <SkeletonPlaceholder /> would need to be agreed upon; logically, it should be different from the one used for video conf.

2. Write unit tests for this new component, to make sure it renders as expected before and after the image has loaded.

3. Use this new component in:

a. app/views/WorkspaceView/ServerAvatar.tsx b. app/containers/markdown/new/Image.tsx c. app/containers/message/Image.tsx

to get the three use cases mentioned in the original issue, respectively. For instance, the lines of code impacted in app/containers/message/Image.tsx would be:

https://github.com/RocketChat/Rocket.Chat.ReactNative/blob/4dfc9c70f3ccd00016edf9c11701fda2b3b5f162/app/containers/message/Image.tsx#L47-L57

We would probably need to update Storyshots to account for these changes (I'm not sure how they behave currently with respect to the images' loading state).

4. Get rid of react-native-image-progress and react-native-progress dependencies, which won't be needed anymore.

We would split this into multiple PRs to facilitate review:

• PR1: steps 1. and 2.,
• PR2: step 3a.,
• PR3: step 3b.,
• PR4: step 3c.,
• PR5: step 4..

What do you think of this approach? Let me know if you see holes in it.

[diegolmello]

@floels Thanks for submitting a detailed proposal. What do you think of doing that on a single PR?

80% of the work is going to be done on the first PR anyway. 2, 3 and 4 would be 10 lines of code at most. PR5 would be a lib removal.

Having all of them on a single PR is going to help us to review and QA.

[floels]

Hi @diegolmello,

Thanks for the response. Sure, I can also do it in a single PR with multiple commits if it's easier.

Let me give it a try then. I should be able to submit something for your review by tomorrow end of day. I might ping you if I need help at some point :)

Cheers, Florian

[ashutosh887]

@diegolmello Let me work on this

[Ansh101112]

Working on this now