Chat scrolls after opening for the 2nd time reported by @thesahindia

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Navigate to a chat
2. Go back
3. Navigate to the same chat again

Expected Result:

The chat shouldn't scroll

Actual Result:

The chat scrolls/jumps

Workaround:

unknown

Platform:

Where is this issue occurring?

• Android

Version Number: 1.2.22-1
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

video_20221102_043919_edit.mp4

az_recorder_20221101_210118.1.mp4

Expensify/Expensify Issue URL:
Issue reported by: @thesahindia
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1667344960109819

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01724eb25130776098
• Upwork Job ID: 1599617199055544320

[melvin-bot]

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

[Christinadobrzyn]

I don't have an Android and I can't replicate it on iOS.

1. Send a message in chat
2. Tap the < arrow in the LHN
3. Tap on the chat again, there's no bouncing

I think this might be similar to these reports

#11962
#12237

So I don't know if this needs to be a new fix - sending to eng for a second opinion

[melvin-bot]

Triggered auto assignment to @techievivek (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[techievivek]

My Android build is broken, but I think this could be related to #11962
#12237

[techievivek]

Just tested this, and there are 2 scenarios here:

1. If we have scrolled to a chat in the history, then re-opening the chat scrolls to the latest message. And in this case, this feels like a smooth transition.
2. If we hadn't scrolled chat when we opened it the first time and then when we re-open chat, there is a noticeable scroll effect.

I am not entirely sure what the expected behaviour is here, so I will ask for help from the team.

[techievivek]

Not overdue, I have started a discussion on slack. But IMO we can fix the Android scrolling behaviour.

[techievivek]

We would like to fix the jumping/drifting effect on Android when one re-opens the chat. Look at the video in the issue description to understand what needs to be fixed.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[techievivek]

Looking for a proposal on this one.

[melvin-bot]

@slafortune, @parasharrajat, @techievivek Whoops! This issue is 2 days overdue. Let's get this updated quick!

[techievivek]

This seems to be happening only on android(native) and not on any other platform. I did invest some time looking into it but couldn't find a solid fix. Not sure how worthy it will be to look more into it, but if you suggest, we can leave it open for proposals or share it internally for someone to have a look. Do let me know what you think. Thanks

[bernhardoj]

@techievivek Ah, my bad. I was checking the code on the main branch and was searching for it. I will take a look at the android implementation.

In the meantime, I tried to remove the autoscrollToTopThreshold property from maintainVisibleContentPosition and it solves the issue. I still don't understand what that property is doing, maybe we can remove it for Android or both as it is an optional property, maybe not.

[techievivek]

It fixes the issue when we remove the property autoscrollToTopThreshold but we would like to keep that property #12363 (comment)

Could you try looking into native code and see if you find something?

I still don't understand what that property is doing

I stumbled upon the same question because I am not able to visualize what these properties do.

[bernhardoj]

I have looked into the native code and still can't find the reason why adding autoscrollToTopThreshold to maintainVisibleContentPosition creating this issue, but I now understand what it is used for after tested it on iOS. When our scroll offset is within the threshold, it will auto scroll to top (or bottom in inverted) when there is a new chat. We combine this property with minIndexForVisible to prevent the list to moving when there is a new chat, but scroll it to top (or bottom) when it is within the scroll threshold. But we set it to 0 which means the auto scroll will never happens.

I believe the property @roryabraham want to keep is the minIndexForVisible as it is the one that prevent the chat list from moving when there is a new chat added.

So, if we want to have a simple solution in the meantime while waiting for the WishList to be published, we can remove the autoscrollToTopThreshold from maintainVisibleContentPosition in BaseInvertedFlatList.js.

[vijeeshin]

159   int currentScroll = mHorizontal ? mScrollView.getScrollX() : mScrollView.getScrollY();

160   for (int i = mConfig.minIndexForVisible; i < contentView.getChildCount(); i++) {

161    View child = contentView.getChildAt(i);

162   float position = mHorizontal ? child.getX() : child.getY();

163     if (position > currentScroll || i == contentView.getChildCount() - 1) {

My observation

After debugging maintainvisiblecontentposition.java it is understood that

Program runs in this pattern

1. Linenumber 159 determines currentscroll to zero
2. Line 160 where i=0, mConfig.minIndexForVisible = 0 and contentView.getChildCount() =19
3. Line 161 View child = contentView.getChildAt(i); which retrives child at 0 position of ReactViewGroup

After Line 161 execution instead of moving to line 162, loop breaks and again start from 159. Then to 160 this time i is not zero but one, then loop runs and breaks at condition is satisfied.

This states that computeTarget runs 2 time before condition is satisfied.

NB: correction need in this line    if (position > = currentScroll || i == contentView.getChildCount() - 1) {

@roryabraham

[JmillsExpensify]

@techievivek Let's remove the hold and see if we can find a reasonable way to solve this ahead of WishList. That's a pretty far out solution.

[techievivek]

@bernhardoj

but scroll it to top (or bottom) when it is within the scroll threshold. But we set it to 0 which means the auto scroll will never happens.

Interesting point, so there is no use in having the property set here; in fact it has some undesirable side effects. Could it be that autoscrollToTopThreshold for android has some bug, and even though it is set to 0, it is causing the drift effect when one re-opens the chat? But I agree if the value is 0, then it doesn't make sense to keep the property. So yes, getting rid of the property seems like one of the solutions here.

@parasharrajat Any thoughts here?

[trjExpensify]

@techievivek can you confirm what happened from here in the issue?

At more than 4 weeks old we expect it to stay internal for someone to champion. It's currently in limbo as neither internal or external. Can you bring a summary to Slack for a discussion? If you haven't got the bandwidth for it anymore with the navigation reboot project, let's source an internal volunteer to carry the torch forward. Thanks!

[techievivek]

@parasharrajat Can you please have a look at the comment put by @bernhardoj here? I would vote we can use that as the solution given it doesn't break anything(which seems like the case here). Though this might be a temporary fix here if we are migrating to WishList in future then the problem will automatically be solved.

But we are also happy to involve Margelo to look into the android implementation for the maintainVisibleContentPosition prop.

Let me know what you think about it.

[parasharrajat]

I will check it today. I am facing 403 issues which is giving me a hard time for 4 days.

[trjExpensify]

@parasharrajat did you not get whitelisted yet? 😕 Please bring this to Slack if not!

[parasharrajat]

No, I requested with my latest IP again.

[techievivek]

Not overdue, waiting for Rajat to share his views on the proposed solution and my comment here.

[parasharrajat]

Need access https://expensify.slack.com/archives/C02NK2DQWUX/p1671506316128929

[roryabraham]

So, if we want to have a simple solution in the meantime while waiting for the WishList to be published, we can remove the autoscrollToTopThreshold from maintainVisibleContentPosition in BaseInvertedFlatList.js.

I think this is okay. Normally we would opt to go for an upstream fix, but realistically we would need help from @janicduplessis to figure that out, and right now I think we'd rather see him dedicate his time to Margelo's incoming replacement for React Native's FlatList.

[JmillsExpensify]

I agree with that. I think this is a totally valid case to go for the simple solution.

[techievivek]

We have decided to close this bug because it appears to be minor and only affects the Android build. Additionally, fixing it would require diving into the native side of the code, which we are not currently interested in exploring as we have more pressing issues and improvements to work on, so we would like to allocate our resources to those tasks.

We hope this bug will be automatically resolved when we migrate to Maregelo's next-gen FlatList implementation, aka WishList. If not, we can always come back and prioritize fixing it.

If you have any comments on this decision, please feel free to add them to this thread, and we can look into it.

[JmillsExpensify]

Thanks, agreed with this approach.