[HOLD for payment 2024-03-04] [$500] iOS-Chat not scrolled to bottom if delete last message by edit > clear comment > apply changes

[lanitochka17]

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

---

Version Number: 1.4.36-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Log in to New Expensify
2. Navigate to a chat with a few messages
3. Hold last message
4. Tap Edit message
5. Clear compose box
6. Tap apply changes

Expected Result:

The edited message should be deleted, and chat history should be scrolled to the very bottom

Actual Result:

The edited message is deleted, but chat history is not scrolled to the bottom. There is one or two messages below

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6367929_1707148265687.OVKO5302.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a3b81b8f8d63b5f3
• Upwork Job ID: 1754543250858106880
• Last Price Increase: 2024-02-05
• Automatic offers:
• jeremy-croff | Contributor | 0

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01a3b81b8f8d63b5f3

[melvin-bot]

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

[melvin-bot]

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

[jeremy-croff]

Proposal

Please re-state the problem that we are trying to solve in this issue.

We don't presreve the scroll position after deleting a message through the edit flow.

What is the root cause of that problem?

Only in IOS, we are maintaing focus on the composer. So the keyboard shows up during edit, shifts the scroll. After delete, we don't have the keyboard, and the scroll position is lost. On other platforms we focus the composer and the keyboard gets shown again, preserving the scroll.

What changes do you think we should make in order to solve the problem?

We have a nice utility to preserve the scroll
reportScrollManager.scrollToIndex(index, false);

Using this utility after the delete draft here:

App/src/pages/home/report/ReportActionItemMessageEdit.tsx

Line 298 in 500dd09

}

will preserve the correct scroll position and address this issue.

But how do we reconcile the logic with the existing scroll listeners on keyboard hide?
I looked closely at the conditions for the existing reportScrollManager.scrollToIndex(index, false); here

App/src/pages/home/report/ReportActionItemMessageEdit.tsx

Lines 293 to 297 in 500dd09

	if (index === 0) {
	const keyboardDidHideListener = Keyboard.addListener('keyboardDidHide', () => {
	reportScrollManager.scrollToIndex(index, false);
	keyboardDidHideListener.remove();
	});

And confirmed that this will run when scrolled all the way to the bottom, and when edited a message to a new message.
By removing the boundaries when that runs, and always running that, it will ensure the messages are always visible. In both that use case, and this use case.

What alternative solutions did you explore? (Optional)#

• Focus the composer and show keyboard after edit on IOS as well.
• Or specific logic to call reportScrollManager.scrollToIndex(index, false); when deleting the message in the edit flow

[HarshMohanSason]

Did you try using the scrollToEnd() method provided by the ScrollView component? That should solve this case if all you need to do is to just scroll at the bottom each time a message edit was successful.

[melvin-bot]

📣 @HarshMohanSason! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[jeremy-croff]

Did you try using the scrollToEnd() method provided by the ScrollView component? That should solve this case if all you need to do is to just scroll at the bottom each time a message edit was successful.

You don't want want to scroll down if you are editing something in history.

[HarshMohanSason]

Did you try using the scrollToEnd() method provided by the ScrollView component? That should solve this case if all you need to do is to just scroll at the bottom each time a message edit was successful.

You don't want want to scroll down if you are editing something in history.

I meant call the function after the delete is completed by setting a flag to know if it has been finished or not.

The solutions you have provided where "Or specific logic to call reportScrollManager.scrollToIndex(index, false); when deleting the message in the edit flow". You can try using a trigger function which is called whenever the keyboard is opened so it saves that position. Then you can have something like

const getSavedScrollPosition = () => {
    return savedScrollPosition;
  };
  
  //and then use this savedScrollPosition in your logic
  
   if (index === 0) { 
     const keyboardDidHideListener = Keyboard.addListener('keyboardDidHide', () => { 
         const scrollPos = getSavedScrollPosition();
         //use this to scroll. 
         keyboardDidHideListener.remove(); 
     }); 
  

[jeremy-croff]

Did you try using the scrollToEnd() method provided by the ScrollView component? That should solve this case if all you need to do is to just scroll at the bottom each time a message edit was successful.

You don't want want to scroll down if you are editing something in history.

I meant call the function after the delete is completed by setting a flag to know if it has been finished or not.

The solutions you have provided where "Or specific logic to call reportScrollManager.scrollToIndex(index, false); when deleting the message in the edit flow". You can try using a trigger function which is called whenever the keyboard is opened so it saves that position. Then you can have something like

const getSavedScrollPosition = () => {
    return savedScrollPosition;
  };
  
  //and then use this savedScrollPosition in your logic
  
   if (index === 0) { 
     const keyboardDidHideListener = Keyboard.addListener('keyboardDidHide', () => { 
         const scrollPos = getSavedScrollPosition();
         //use this to scroll. 
         keyboardDidHideListener.remove(); 
     }); 
  

I'm trying to understand, but the root cause of the current IOS bug is that the existing scroll is not working because keyboardDidHide actually never occurs. Since the composer doesn't get refocused. That's why I proposed to either call a scroll to current position after delete, or focus on the composer.

[stephanieelliott]

Hey @allroundexperts would you mind weighing in on the proposals here?

[allroundexperts]

I think we should focus the composer and show the keyboard again on iOS as well like we do on other platforms to maintain consistency. As such, @jeremy-croff's alternate proposal looks good to me.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[NikkiWines]

Yep, agreed - let's go with consistency 👍

[melvin-bot]

📣 @jeremy-croff 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer 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 📖

[jeremy-croff]

@NikkiWines

I'm reviewing the composer consistency idea and have the following considerations:
We have specific code to not focus the composer on touch devices. The comments specify because it's jarring.

I was wrong saying not re-focusing is IOS specific, there are actually conditions seperate for touch screen devices like is my report empty then show the keyboard so when testing on android I was missing these conditions when comparing.

Showing the keyboard after edit actually did not fix this bug after implementing this locally and the root cause is specified in the comment below.

Sorry for the complication. See below comment for the root cause and why this is only reproducible on IOS.

[jeremy-croff]

While developing I identified a one liner that caused this issue:

updating export default setShouldShowComposeInputKeyboardAwareBuilder('keyboardWillHide');
to export default setShouldShowComposeInputKeyboardAwareBuilder('keyboardDidHide');

App/src/libs/setShouldShowComposeInputKeyboardAware/index.ios.ts

Line 5 in bfae5b2

export default setShouldShowComposeInputKeyboardAwareBuilder('keyboardWillHide');

Fixes this issue, and is the root cause. Only IOS does it on willHideKeyboard.

It was done specifically for #29211 (comment)

This impacts the timing between the scroll and the insertion of the composer element which is pushing the text off.

In a branch I have made the change back to didHideKeyboard to fix the scroll on IOS
And added useEffect(() => ()=> ComposerActions.setShouldShowComposeInput(true), []) to the unmount of the ReportActionItemMessageEdit to force the composer to spring back quickly as opposed to after keyboard hide.

This seems to work the best.

[jeremy-croff]

@allroundexperts I'm looking for approval on the information above. I have a draft PR open and can finalize the checklist if this looks good.

[jeremy-croff]

@allroundexperts bumping this.

[allroundexperts]

That's fine @jeremy-croff. Please open the PR so I can test it.

[jeremy-croff]

That's fine @jeremy-croff. Please open the PR so I can test it.

I have opened the draft for review so you can test it.
I will be in office, but at the end of my day in approx 12 hrs I can go through the checklist thoroughly and test.

[jeremy-croff]

Expecting to continue work on this in 12 hours

[jeremy-croff]

Ready for review!

[stephanieelliott]

Hey @allroundexperts looks like this is waiting on your review!

[stephanieelliott]

PR was deployed to staging

[stephanieelliott]

This was deployed to prod on 2/28, seems the automation didn't work so am updating manually

[stephanieelliott]

Summarizing payment on this issue:

• Contributor: @jeremy-croff $500 PAID via Upwork
• Contributor+: @allroundexperts $500 Please request via ND

Upwork job is here: https://www.upwork.com/jobs/~1737876297360310272"

[JmillsExpensify]

$500 approved for @allroundexperts based on summary.