[$250] Chat scrolls back to the last message on opening the keyboard reported by @Puneet-here

[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. Go to any chat
2. Scroll the chat so that you can see the old messages
3. Tap on the input to open the keyboard

Expected Result:

The chat shouldn't scroll back to the last message

Actual Result:

The chat scroll back to the last message

Workaround:

unknown

Platform:

Where is this issue occurring?

• iOS
• Android

Version Number: 1.2.21-4
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:
Expensify/Expensify Issue URL:
Issue reported by: @Puneet-here
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1666912513143069

Screen.Recording.2022-10-28.at.4.31.07.AM.mov

az_recorder_20221028_104040.mp4

View all open jobs on GitHub

[melvin-bot]

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

[miljakljajic]

Assigning an engineer to triage - can this be worked on by an external contributor?

[melvin-bot]

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

[jasperhuangg]

Looks like a good candidate for the External label.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[TwizzyIndy]

Proposal

We only have to remove the keyboardEvent listener on ReportActionsView. It always trigger the ReportScrollManager.scrollToBottom when the keyboardDidShow occurs. I think that would be fine.

App/src/pages/home/report/ReportActionsView.js

Lines 104 to 114 in 25c3060

	this.keyboardEvent = Keyboard.addListener('keyboardDidShow', () => {
	if (!ReportActionComposeFocusManager.isFocused()) {
	return;
	}
	ReportScrollManager.scrollToBottom();
	});
	if (this.getIsReportFullyVisible()) {
	this.openReportIfNecessary();
	}
	}

Simulator.Screen.Recording.-.iPhone.13.-.2022-11-03.at.16.25.25.mp4

[michaelhaxhiu]

Heya @miljakljajic just dropping a note as a reminder to keep the pressure on to find a contributor and get this one closed out :)

Can we export this one to Upwork since it's external? That'll help increase public visibility on the job.

Also @rushatgabhane are you able to review the proposal today or tomorrow?

[rushatgabhane]

@TwizzyIndy Thanks for your proposal, I'm curious to know if issue #3545 reappears because we're effectively reverting the PR for it.

[TwizzyIndy]

Thanks for reviewing bro @rushatgabhane. I want to update my proposal.

Proposal

We don't really have to remove the keyboardEvent here. We should check whether the current flatlist is at the bottom or not. We shouldn't scroll to the bottom if it is at the bottom. There is currentScrollOffset in ReportActionView which reflects the current scroll position. So we can check it like this. This looks also fine with #3545

        this.keyboardEvent = Keyboard.addListener('keyboardDidShow', () => {
+            if (!ReportActionComposeFocusManager.isFocused() || this.currentScrollOffset !== 0) {
                return;
            }
            ReportScrollManager.scrollToBottom();
        });

Simulator.Screen.Recording.-.iPhone.13.-.2022-11-04.at.14.42.28.mp4

Simulator.Screen.Recording.-.iPhone.13.-.2022-11-04_480.mov

[miljakljajic]

Job posted to Upwork.

[jasperhuangg]

@TwizzyIndy I like your proposal and am inclined to accept it, but can you modify it a bit to include an explanation of how the logic in this condition would fix the problem? Ping me when you do. Thanks!

if (!ReportActionComposeFocusManager.isFocused() || this.currentScrollOffset !== 0)

[TwizzyIndy]

Thank you. @jasperhuangg. I would like to add some explanation like this below.

        this.keyboardEvent = Keyboard.addListener('keyboardDidShow', () => {
            // When the keyboard shows up and then
            // if not focused on the report action composer or the current scroll position is not at the bottom
            // we won't scroll to the bottom here.
            if (!ReportActionComposeFocusManager.isFocused() || this.currentScrollOffset !== 0) {
                return;
            }
            ReportScrollManager.scrollToBottom();
        });

[miljakljajic]

what do you think @rushatgabhane @jasperhuangg - are you happy with the suggestion @TwizzyIndy has provided above?

[melvin-bot]

@miljakljajic, @rushatgabhane, @jasperhuangg Eep! 4 days overdue now. Issues have feelings too...

[miljakljajic]

Seems like the PR has merged, shall I close?

[Puneet-here]

@miljakljajic, the reporting bonus is pending.

[miljakljajic]

Ah my apologies, Puneet! I forgot that you reported it. I've hired you in Upwork for the job.

[Puneet-here]

Thanks! I have accepted the offer.

[marcaaron]

@jasperhuangg, I'm not sure if I agree with the outcome here. Messages on iOS always scrolls you to the bottom when you pop open the input. I prefer that behavior because if you are composing a message you'll want to see the latest message for context.

[JmillsExpensify]

I'm with @marcaaron. I'm not sure if I agree with this bug report. Like clearly there isn't a right way here, it's just a preference. Some apps do it one way and other apps do it another way. As a result, this issue is wading into product design rather than bug fixing. I personally think we should close it until we have a larger conversation about which design is right.

[jasperhuangg]

agree @JmillsExpensify thanks for the clarify here, definitely felt out of my depth with product design stuff. I also talked to a few different people about this issue and it seems like everyone has a different preference.

@marcaaron Did not realize that that was the behavior for Messages on iOS, thanks for pointing that out.

I prefer that behavior because if you are composing a message you'll want to see the latest message for context.

At least when I'm scrolled up in a chat, I'm most likely replying to the message I'm scrolled up to, not the latest one that I can't see on my screen. If you want another mechanism to scroll down to the bottom, perhaps we could add a button with an arrow on it that scrolls us down?

Regardless, I'll apply the Planning label to this and start up a chat in #expensify-open-source.

[marcaaron]

I'm most likely replying to the message I'm scrolled up to

Aren't you always kind of replying to the entire chat / last message and not some historical message that you are scrolled up to? Once we add threads then you can reply to a specific message.

[jasperhuangg]

@marcaaron Fair point, I think threads would make this behavior make a lot more sense. I guess most chat apps that include scrolling to the bottom behavior also include some sort of "reply to message" functionality, thanks for that.

I dropped a message in #expensify-open-source here, let's continue discussion there?

[jasperhuangg]

Closing as per conclusion from discussion here

[Puneet-here]

@jasperhuangg, this still needs to be settled up. Can we keep this open?

[Puneet-here]

@jasperhuangg bump

[JmillsExpensify]

@Puneet-here Invited you via Upwork for reporting. Job is also here: https://www.upwork.com/jobs/~01aa7c375e34f4cbd5. Can you apply and we'll get this handled ponto?

[jasperhuangg]

apologies @Puneet-here totally didn't realize! Yes, please apply on Upwork as Jason mentioned, thanks again for the bump

[JmillsExpensify]

@Puneet-here Still waiting for you to accept in Upwork.

[Puneet-here]

@Puneet-here Still waiting for you to accept in Upwork.

Applied, thanks!

[jasperhuangg]

Not overdue, just waiting for @Puneet-here to get paid out and we can close this out

cc @JmillsExpensify looks like he's all set up to get paid!

[JmillsExpensify]

Offer sent to @Puneet-here Waiting for him to accept and then someone can pay him out. @miljakljajic or @jasperhuangg do either of you mind doing that today/tomorrow? I'm OOO this week.

[Puneet-here]

Accepted

[miljakljajic]

Paying it!

[miljakljajic]

All paid. Closing!