[HOLD for payment 2024-07-10] [HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] Chat - Compose box is hidden when editing multiple line break message

[izarutskaya]

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

---

Version Number: v1.4.64-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4503695
Email or phone of affected tester (no customers): [email protected]
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Pre-requisite: user must be logged in.

1. Go to any chat.
2. Send a message with 10 line breaks.
3. Tap and hold on the recently sent message.
4. Tap on "Edit comment".
5. Verify the compose box is partially hidden, the chat does not auto-scroll to the bottom.

Expected Result:

The page should auto-scroll to the bottom and the compose box should be fully visible without manual scroll.

Actual Result:

The page is not auto-scrolled to the bottom, the compose box is partially hidden, manual scroll is required.

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

Bug6457792_1713810082838.Vbjf1205_1_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0179a22c17165154c1
• Upwork Job ID: 1782955414960959488
• Last Price Increase: 2024-05-29
• Automatic offers:
• rojiphil | Reviewer | 102547928
• suneox | Contributor | 102547931

Issue Owner

Current Issue Owner: @trjExpensify

[melvin-bot]

Triggered auto assignment to @trjExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[izarutskaya]

We think this issue might be related to the #vip-vsb.

[trjExpensify]

I can reproduce it. Couldn't find a recent issue, looks like @kosmydel fixed it once before here in Oct 2023: #24502

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0179a22c17165154c1

[melvin-bot]

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

[suneox]

Proposal

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

Chat - Compose box is hidden when editing multiple line break message

What is the root cause of that problem?

When editing a message item on report item list will be focus and scrollToIndex immediate, but the keyboard shown 100% after the list item scrollToIndex and the height of report item will resize to smaller so the keyboard will overlap the edit composer

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

We will be waiting the keyboard open 100% before scrollToIndex. We can use InteractionManager.runAfterInteractions and requestAnimationFrame to wait the keyboard opened then scroll to index
Update onFocus function to

    onFocus={() => {
        setIsFocused(true);
+       InteractionManager.runAfterInteractions(() => {
+           requestAnimationFrame(() => {
                reportScrollManager.scrollToIndex(index, true);
+           });
+       });
        setShouldShowComposeInputKeyboardAware(false);
        ...

Same solution with this and this

Before fix

Screen.Recording.2024-04-24.at.12.39.49.mov

After fix

Screen.Recording.2024-04-24.at.12.40.57.mov

What alternative solutions did you explore? (Optional)

[suneox]

I can reproduce it. Couldn't find a recent issue, looks like @kosmydel fixed it once before here in Oct 2023: #24502

@trjExpensify This issue still happens when you edit the last item has multiple lines

Screen.Recording.2024-04-24.at.12.39.49.mov

[tienifr]

Proposal

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

The page is not auto-scrolled to the bottom, the compose box is partially hidden, manual scroll is required.

What is the root cause of that problem?

We're running scrollToIndex right after the edit composer is focused. At that time, the keyboard is still showing and when it 's shown totally, the keyboard will be overlap with the composer

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

We need to wait for keyboard is shown totally. To do that we shouldn't lean on runAfterInteractions or requestAnimationFrame because they're not enough to make sure the keyboard is shown 100%. Instead of that we should use keyboardDidShow event

const keyboardDidShowListener = Keyboard.addListener('keyboardDidShow', (e) => {
                                    reportScrollManager.scrollToIndex(index, true);
                                    keyboardDidShowListener.remove()
                                });

What alternative solutions did you explore? (Optional)

NA

Result

Screen.Recording.2024-04-24.at.16.18.12.mov

[kosmydel]

I can reproduce it. Couldn't find a recent issue, looks like @kosmydel fixed it once before here in Oct 2023: #24502

The mentioned issue was related to mWeb only, not native. At first glance, the root cause of this issue is different.

[suneox]

I think the RCA and solution on this proposal is inherited and duplicated with my proposal is we have to wait keyboard show

however, my chosen option for solution wait keyboard show based on a lot of code base also use runAfterInteractions to allows JavaScript animations to run smoothly avoid many transitions run the same time, and the event keyboardDidShow doesn't support for mWeb

[tienifr]

Screen.Recording.2024-04-25.at.11.49.38.mov

It works well on mweb. As I said in my proposal, runAfterInteractions can not make sure the keyboard is shown totally

[suneox]

react-native-web doesn't implement Keyboard event

I have tried keyboard event before using runAfterInteractions and it doesn't work for mWeb

Screen.Recording.2024-04-25.at.23.33.06.mov

My solution using runAfterInteractions for scroll to item the same with this one, this one ... and flatlist also apply requestAnimationFrame for scroll to item that all the reason i using runAfterInteractions and requestAnimationFrame to make sure no frame update that also mean the keyboard opened it also avoid multiple transition run same time

[sword1202]

If there is multi lines text, it's impossible to edit on iPhone.

bug_cannotEditMultiLine.mp4

[trjExpensify]

@rojiphil are you going to review these proposals?

[rojiphil]

@rojiphil are you going to review these proposals?

@trjExpensify Yes. Will review and share an update today.

[rojiphil]

react-native-web doesn't implement Keyboard event

@tienifr What are your thoughts on this concern?

[rojiphil]

runAfterInteractions can not make sure the keyboard is shown totally

@suneox And what are your thoughts on this concern?

[suneox]

@suneox And what are your thoughts on this concern?

@rojiphil I also explain in my comment the reason I was using runAfterInteractions with requestAnimationFrame to make sure it can work for all platform instead of the keyboard even.

My solution using runAfterInteractions for scroll to item the same with this one, this one ... and flatlist also apply requestAnimationFrame for scroll to item that all the reason i using runAfterInteractions and requestAnimationFrame to make sure no frame update that also mean the keyboard opened it also avoid multiple transition run same time

[rojiphil]

@suneox @tienifr On further investigation, I could reproduce the problem even without the keyboard on my iOS simulator. So how can scrolling early even before the Keyboard shows up completely be the root cause here?

40767-scroll-issue.mp4

[suneox]

@suneox @tienifr On further investigation, I could reproduce the problem even without the keyboard on my iOS simulator. So how can scrolling early even before the Keyboard shows up completely be the root cause here?

40767-scroll-issue.mp4

You must enable keyboard by shortcut CMD+k

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.81-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• handle focus after edit report item #42965

If no regressions arise, payment will be issued on 2024-06-18. 🎊

For reference, here are some details about the assignees on this issue:

• @rojiphil requires payment automatic offer (Reviewer)
• @suneox requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@rojiphil] The PR that introduced the bug has been identified. Link to the PR:
• [@rojiphil] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@rojiphil] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@rojiphil] Determine if we should create a regression test for this bug.
• [@rojiphil] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@trjExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.82-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• handle focus after edit report item #42965

If no regressions arise, payment will be issued on 2024-06-20. 🎊

For reference, here are some details about the assignees on this issue:

• @rojiphil requires payment automatic offer (Reviewer)
• @suneox requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@rojiphil / @suneox] The PR that introduced the bug has been identified. Link to the PR:
• [@rojiphil / @suneox] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@rojiphil / @suneox] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@rojiphil / @suneox] Determine if we should create a regression test for this bug.
• [@rojiphil / @suneox] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@trjExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[trjExpensify]

👋 checklist time, please!

[suneox]

@rojiphil you're a reviewer for this issue so could you please complete a check list?

[rojiphil]

• [@rojiphil] The PR that introduced the bug has been identified. Link to the PR: Offending PR
• [@rojiphil] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: Added comment
• [@rojiphil] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: Not Required. Existing checklist is good enough to capture such issues.
• [@rojiphil] Determine if we should create a regression test for this bug. : Given the spooky nature of this issue, it is better to have a regression test case to ensure this does not occur again.
• [@rojiphil] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

1. Go to any chat.
2. Send a message with 10 line breaks.
3. Tap and hold on the recently sent message.
4. Tap on "Edit comment".
5. Verify that the page auto-scrolls to the bottom and the compose box is fully visible without manual scroll.

[rojiphil]

👋 checklist time, please!

@trjExpensify Completed BZ checklist and accepted the offer too. Thanks.

[trjExpensify]

Thanks! Regression test issue created for Applause.

Payment summary as follows:

• $250 to @suneox for the fix
• $250 to @rojiphil for the C+ review

Paid you both, closing!

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.3-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• handle focus after edit report item #42965

If no regressions arise, payment will be issued on 2024-07-10. 🎊

For reference, here are some details about the assignees on this issue:

• @rojiphil requires payment automatic offer (Reviewer)
• @suneox requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@rojiphil / @suneox] The PR that introduced the bug has been identified. Link to the PR:
• [@rojiphil / @suneox] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@rojiphil / @suneox] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@rojiphil / @suneox] Determine if we should create a regression test for this bug.
• [@rojiphil / @suneox] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@trjExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: