[HOLD for payment 2023-10-25] [$1000] Web - App maintains focus but does not scroll to bottom at cursor position on reload

[kbecciv]

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. Open the app
2. Open any report
3. Send big message that enables scroll
4. Reload and observe that focus is on composer but it does not scroll to cursor position
5. Click on any user profile to open details page and reload
6. Click on back and observe that now app scrolls to bottom to cursor position

Expected Result:

App should scroll to cursor position if focus is maintained on composer box

Actual Result:

App maintains focus but does not scroll to bottom at cursor position in compose box

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.67.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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

scroll.to.bottom.not.working.for.big.messages.1.mp4

Recording.4382.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694063310399689

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01048bbbaff6031064
• Upwork Job ID: 1701240589171867648
• Last Price Increase: 2023-09-28
• Automatic offers:
• jjcoffee | Reviewer | 27014842
• jeet-dhandha | Contributor | 27014844
• dhanashree-sawant | Reporter | 27014846

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01048bbbaff6031064

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

Triggered auto assignment to @bfitzexpensify (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 - @jjcoffee (External)

[binary3oul]

Proposal

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

App maintains focus but does not scroll to bottom at cursor position on reload.

What is the root cause of that problem?

When we refresh the page, Composer component saves the position of the cursor whereas don't save scrollTop property.
In Composer component, we missed to set scrollTop propery of textInput.

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

We need to add a line to set scrollTop property like this:
textInput.current.scrollTop = textInput.current.scrollHeight;

in this line in useEffect hook.

Totally, the code looks like

useEffect(() => {
textInput.current.scrollTop = textInput.current.scrollHeight;

What alternative solutions did you explore? (Optional)

we can also use textInput.current.scrollTop = selectionProp.start

[usafhassan]

Proposal

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

App maintains focus but does not scroll to bottom at cursor position on reload

What is the root cause of that problem?

Its refreshing, that is why it is losing the state

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

We need to add a single line of code to set the value of scrollTop

textInput.current.scrollTop = textInput.current.scrollHeight;

The updated code will look like this

 // rest of the code
 useEffect(() => {
    if (textareaRef.current && msgText) {
      textareaRef.current.scrollTop = textareaRef.current.scrollHeight;
    }
  }, []);
 // rest of the code

What alternative solutions did you explore? (Optional)

Contributor details
Your Expensify account email: usafhassan@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0141ec503adff92d50

[melvin-bot]

📣 @usafhassan! 📣
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. 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.
2. 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.
3. 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>

[usafhassan]

Contributor details
Your Expensify account email: usafhassan@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0141ec503adff92d50

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[jjcoffee]

@usafhassan Welcome 👋 and thanks for your proposal! Please read our contributing guide and use the proposal template there.

For both proposals I think it'd be nice to add to the RCAs why steps 5 & 6 work to set the correct scroll position. @usafhassan @binary3oul

[binary3oul]

Updated Proposal

[jjcoffee]

@binary3oul Thanks, I'm still interested in why steps 5 & 6 work to set the correct scroll position though 😄

[binary3oul]

When we refresh the page, Composer component saves the position of the cursor whereas don't save scrollTop property.
In Composer component, we missed to set scrollTop propery of textInput.

[jjcoffee]

Specifically why does this step currently work though:

Click on back and observe that now app scrolls to bottom to cursor position

[usafhassan]

@jjcoffee
The step 5 and 6 works because when we go back to the report page where we are writing a long message,
that page is still maintaining it's state (or getting from a global store/redux store), as we did not reload that page(report page),
even though we reloaded the details page, it does not affect the state of report page and we still go to cursor position.

P.S. I have updated my proposal according to the template

[jjcoffee]

@usafhassan Thanks, but my point is even if I scroll up to the top of the message and then perform the same steps (open details page, reload, go back), the input is scrolled to the bottom. I think it'd be useful to understand what's happening there as presumably we could use the same on reload to keep things nice and clean!

[usafhassan]

@jjcoffee when we close the side menu, the useEffect runs which calls the focus() function

[usafhassan]

@jjcoffee also we can add another dependency variable for reload and set it to true in initial state, then add it in dependency array of useEffect where we are setting the scroll position, this will set it to correct position on reload

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[jjcoffee]

Reviewing tomorrow!

[melvin-bot]

📣 @jjcoffee 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @jeet-dhandha 🎉 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 📖

[melvin-bot]

📣 @dhanashree-sawant 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[jeet-dhandha]

@jjcoffee - There's an issue with Mobile Safari its auto-focusing by default. This is happening in main also. Should we handle it in a different PR ?

[jeet-dhandha]

All mobile devices don't focus, when its not an empty chat.

MobileSafari.mp4

[jjcoffee]

@jeet-dhandha Yeah I would say if there's a general issue on native devices that the composer isn't focused (whether intentional or not), that would be out of scope for this issue. Feel free to report it!

[dylanexpensify]

@jeet-dhandha @jjcoffee mind giving us an update?

[jjcoffee]

@dylanexpensify PR is under review - just needed quite a bit of back and forth to get the code in shape!

[jeet-dhandha]

@jjcoffee I have asked u to review again but seems like u r busy with other tasks.

CC: @dylanexpensify

[melvin-bot]

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

• when @jeet-dhandha got assigned: 2023-10-04 09:36:40 Z
• when the PR got merged: 2023-10-17 15:36:13 UTC
• days elapsed: 9

On to the next one 🚀

[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.3.86-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: auto focus and scroll to bottom on large-text in composer #28790

If no regressions arise, payment will be issued on 2023-10-25. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @jjcoffee requires payment offer (Reviewer)
• @jeet-dhandha requires payment offer (Contributor)
• @dhanashree-sawant requires payment offer (Reporter)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[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:

• [@jjcoffee] The PR that introduced the bug has been identified. Link to the PR:
• [@jjcoffee] 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:
• [@jjcoffee] 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:
• [@jjcoffee] Determine if we should create a regression test for this bug.
• [@jjcoffee] 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.
• [@bfitzexpensify / @dylanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[dylanexpensify]

payment tomorrow!

[jeet-dhandha]

Bump for payment

[jjcoffee]

• The PR that introduced the bug has been identified. Link to the PR: N/A - we've never handled this
• 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: N/A
• 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: N/A
• Determine if we should create a regression test for this bug. Yes
• 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.

Regression test proposal

1. Open any report
2. Enter a long enough text to make composer take full height and scroll bars appear
3. Refresh the page of that report.
4. Verify that the composer is scrolled to the bottom for all platforms except iOS.

Do we agree 👍 or 👎

[bfitzexpensify]

Payments complete - will review the proposed regression steps tomorrow

[bfitzexpensify]

Regression steps look good, opened an issue to get it updated. We're all done here, thanks everyone