[$1000] Skeleton-view is right oriented when scrolling up and loading new messages on Android

[yuwenmemon]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Have a chat with 100+ comments on it
2. On Android, scroll up quickly, so that you cause the chat to need to load more chats

Expected Result:

The skeleton view that indicated we're loading more chats is left-oriented like on other platforms.

Actual Result:

The skeleton view is right-oriented

Workaround:

N/A

Platform:

Where is this issue occurring?

• Android

Version Number: 1.2.40
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): N/A
Logs: N/A
Notes/Photos/Videos:
https://user-images.githubusercontent.com/4741899/207949730-7c6c4800-24b1-427b-9590-7f296d098353.mp4

Expensify/Expensify Issue URL: N/A
Issue reported by: @yuwenmemon
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1671130333575879

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01383ef4f4c88c4510
• Upwork Job ID: 1603522970016489472
• Last Price Increase: 2022-12-15

[melvin-bot]

Triggered auto assignment to @kevinksullivan (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Current assignee @kevinksullivan is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01383ef4f4c88c4510

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External)

[melvin-bot]

Triggered auto assignment to @sketchydroide (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[bernhardoj]

Proposal

The inverted FlatList on Android set the inverted to false and use translate style to make it inverted.

App/src/components/InvertedFlatList/index.android.js

Lines 6 to 24 in 3620bf2

	const InvertedCell = props => (
	// eslint-disable-next-line react/jsx-props-no-spreading
	<View {...props} style={styles.invert} />
	);
	export default forwardRef((props, ref) => (
	<BaseInvertedFlatList
	// eslint-disable-next-line react/jsx-props-no-spreading
	{...props}
	ref={ref}
	// Manually invert the FlatList to circumvent a react-native bug that causes ANR (application not responding) on android 13
	inverted={false}
	style={styles.invert}
	verticalScrollbarPosition="left" // We are mirroring the X and Y axis, so we need to swap the scrollbar position
	CellRendererComponent={InvertedCell}
	/>
	));

As you can see, we also set the CellRendererComponent and apply the styles.inverted to the cell. So, what we need to do is to also apply the style to the footer component.

<BaseInvertedFlatList
    // eslint-disable-next-line react/jsx-props-no-spreading
    {...props}
    ref={ref}

    // Manually invert the FlatList to circumvent a react-native bug that causes ANR (application not responding) on android 13
    inverted={false}
    style={styles.invert}
    verticalScrollbarPosition="left" // We are mirroring the X and Y axis, so we need to swap the scrollbar position

    CellRendererComponent={InvertedCell}
+   ListFooterComponentStyle={styles.invert}
/>

Result

328544.t.mp4

[kganakga]

Proposal

the skeleton view was right-oriented because the BaseInvertedFlatList component was configured to invert the layout of the FlatList. This was done by setting the inverted prop to false and applying the styles.invert style to the FlatList component.
this line of code caused the skeleton view to be right-oriented:
inverted={false}
style={styles.invert}
By setting the inverted prop to false, the layout of the FlatList was inverted, which caused the skeleton view to be right-oriented. The styles.invert style was also applied to further invert the appearance of the FlatList.

To fix the issue and display the skeleton view in a left-oriented manner, you can set the inverted prop to true and remove the styles.invert style. This will restore the default, non-inverted layout of the FlatList and display the skeleton view in a left-oriented manner.

I hope this helps to clarify the cause of the issue and how to fix it. I have written code that can solve it.

[mananjadhav]

Thanks for the proposal folks. @bernhardoj proposal looks good here.

I am also tagging @mountiny and @Beamanator for second review as this broken when we moved InvertedFlatList to index.android.js in this PR #12820.

@sketchydroide all yours.

[Beamanator]

That sooounnnds good to me, what do you think @mountiny ?

Also I think it could be good to get @hannojg 's opinion here since they've been working on a lot of Android fixes lately, and they're the author of that PR that caused this issue. @hannojg would you mind taking a look at this proposal which could fix this issue that was probably caused by your PR? :D

[hannojg]

The proposal from @bernhardoj looks good to me! Setting the inverted back to true is currently not an option due to a bug in the Android framework on Android 13.

Good catch!

[0xmiros]

Proposal

https://github.com/Expensify/App/blob/main/src/components/InvertedFlatList/index.android.js

InvertedFlatList should be perfect as a common component, so invert should be applied to all possible components, not only footer.

+ const renderEmptyComponent = (props) => {
+    const ListEmptyComponent = props.ListEmptyComponent;
+    if (!ListEmptyComponent) {
+        return null;
+    }
+    return (
+        <View style={[styles.invert, props.ListEmptyComponentStyle]}>
+            <ListEmptyComponent />
+        </View>
+    );
+ };

export default forwardRef((props, ref) => (
    <BaseInvertedFlatList
        // eslint-disable-next-line react/jsx-props-no-spreading
        {...props}
        ref={ref}

        // Manually invert the FlatList to circumvent a react-native bug that causes ANR (application not responding) on android 13
        inverted={false}
        style={styles.invert}
        verticalScrollbarPosition="left" // We are mirroring the X and Y axis, so we need to swap the scrollbar position

        CellRendererComponent={InvertedCell}
+       ListEmptyComponent={renderEmptyComponent(props)}
+       ListFooterComponentStyle={[styles.invert, props.ListFooterComponentStyle]}
+       ListHeaderComponentStyle={[styles.invert, props.ListHeaderComponentStyle]}
    />
));

propTypes definition can be done in PR stage

[s77rt]

Shouldn't we fix the bug in RN instead of the style workaround?
@hannojg Can you link the RN bug you mentioned on your PR?

[0xmiros]

As @hannojg commented here, I think it's android native sdk issue should be fixed by Google team
facebook/react-native#35350 (comment)

[sketchydroide]

I think we can go with @bernhardoj suggestion then

[melvin-bot]

📣 @bernhardoj You have been assigned to this job by @sketchydroide!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[0xmiros]

@sketchydroide just to confirm if you checked my proposal already

Though ListFooterComponentStyle prop is not used yet, that prop might be set for future use.
So if we just replace ListFooterComponentStyle with styles.invert, ListFooterComponentStyle prop value set in high level component is skipped in android.
So the correct fix should be ListFooterComponentStyle={[styles.invert, props.ListFooterComponentStyle]}

Same applies to header, empty components

[bernhardoj]

Applied on Upwork. I will open the PR today or tomorrow.

[hannojg]

Do we have a video of the bug occurring? While I agree with @bernhardoj approach to apply the style to other parts of the flatlist (instead of inverting the list by using the invert prop), I am not sure if the footer thing actually fixes the whole issue? The proposal might need another pair of eyes 😅

[0xmiros]

@hannojg Current app uses InvertedFlatList in ReportActionsList with ListFooterComponent props set. No other usages yet.
So footer thing fixes the issue for now.
But in the future, this component usage can be expanded and there might need of ListHeaderComponent or ListEmptyComponent prop values set so in those cases, header and empty things should be fixed as well.

[mountiny]

Nice find! I think @0xmiroslav proposal would be great once we start using the InvertedFlatList in other places too! For now the first solution works fine and is quick and simple.

[bernhardoj]

I have opened the PR, however I'm currently unable to test it on iOS Safari because of the 403 sign in problem.

[sketchydroide]

the PR has been merged

[sketchydroide]

still in QA at the moment as far as I can tell, I think QA is on holidays, so it might take some time

[sketchydroide]

ok this is already in prod, I don't know if everyone has been payed yet, @kevinksullivan when come back next week can you check that?

[mananjadhav]

This was deployed to production 5 days back so should be ready for payout in 2 days! Title isn't updated but payment is pending.

[sketchydroide]

thanks @mananjadhav

[mananjadhav]

@sketchydroide @kevinksullivan This is ready for payout today!

[sketchydroide]

@kevinksullivan maybe you can take of this when you come in tomorrow

[kevinksullivan]

Thanks for your patience @mananjadhav @bernhardoj . I just sent offers to you both. Let me know when you've accepted and I'll get this paid.

[mananjadhav]

Accepted @kevinksullivan. Thanks for helping out here.

[bernhardoj]

@kevinksullivan Accepted.

[mananjadhav]

@kevinksullivan Is the contributor paid out too? Anything else pending here or are we good to close this one out?

[bernhardoj]

I already get paid.

[mananjadhav]

Thanks for confirming @bernhardoj. @kevinksullivan anything else pending here?

[sketchydroide]

if both of you ae payed I think we can close it