[HOLD for payment 2024-12-16] [$250] Chat - Compose box stays in the middle when reduced with a several lines message

[IuliiaHerets]

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: 9.0.39-0
Reproducible in staging?: Y
Reproducible in production?: Y
Issue reported by: Applause Internal Team

Action Performed:

1. Open the staging.new.expensify.com website.
2. Open any chat.
3. On compose box, start writing a message adding new lines until a scrollbar can be seen.
4. Tap on the "Expand" button.
5. Keep adding lines to the message until a scrollbar can also be seen with the compose box expanded.
6. Reduce the compose box size.
7. Verify if you are scrolled to the bottom when the compose box is reduced.

Expected Result:

When the compose box with a written message of several lines is reduced in size, the user should be scrolled to the bottom of it.

Actual Result:

When the compose box with a written message of several lines is reduced in size, the user is not scrolled to the bottom. The compose box is displayed in the middle of the message.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6609918_1726836461310.Compose_Reduce.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021838742813045152105
• Upwork Job ID: 1838742813045152105
• Last Price Increase: 2024-10-02
• Automatic offers:
• rayane-djouah | Reviewer | 104312306
• FitseTLT | Contributor | 104312308

Issue Owner

Current Issue Owner: @JmillsExpensify

[melvin-bot]

Triggered auto assignment to @anmurali (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.

[IuliiaHerets]

We think that this bug might be related to #wave-collect - Release 1

[IuliiaHerets]

@anmurali FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[FitseTLT]

Proposal

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

Chat - Compose box stays in the middle when reduced with a several lines message

What is the root cause of that problem?

When we toggle isComposerFullSize there is a code that sets the scrollTop to the previous scrollTop (the scrollTop in the previous isComposerFullSize state)

App/src/components/Composer/index.tsx

Lines 268 to 271 in 25450d9

	// eslint-disable-next-line react-compiler/react-compiler
	textInput.current.scrollTop = prevScroll;
	// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
	}, [isComposerFullSize]);

so when the when the composer was full size the scroll top had a smaller value (that is because the composer had greater height and needs to scroll with smaller value to show the bottom of the composer than when it is non-full size) so when we switch it to non-full size that small value of scrollTop will not be enough to show the bottom of the composer.

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

We need to set the scrollTop in such a way that the content on the bottom of the composer will be persitent on toggling isComposerFullSize so: we should add height state to hold previous clientHeight the change

App/src/components/Composer/index.tsx

Line 232 in 25450d9

setPrevScroll(textInput.current.scrollTop);

            setPrevScroll(textInput.current.scrollTop);
            setPrevHeight(textInput.current.clientHeight);

App/src/components/Composer/index.tsx

Lines 268 to 271 in 25450d9

	// eslint-disable-next-line react-compiler/react-compiler
	textInput.current.scrollTop = prevScroll;
	// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
	}, [isComposerFullSize]);

 if (!textInput.current || prevScroll === undefined || prevHeight === undefined) {
            return;
        }
        // eslint-disable-next-line react-compiler/react-compiler
        textInput.current.scrollTop = prevScroll + prevHeight - textInput.current.clientHeight;
       

What alternative solutions did you explore? (Optional)

We if we want the center of the content persistent we can change

        textInput.current.scrollTop = prevScroll + prevHeight / 2 - textInput.current.clientHeight / 2 ;

[melvin-bot]

@anmurali Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

@anmurali, @rayane-djouah Eep! 4 days overdue now. Issues have feelings too...

[rayane-djouah]

Sorry for the delay! will review this one later today / tomorrow

[QichenZhu]

Proposal

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

Composer's caret scroll position is not maintained after resuming from expanded mode.

What is the root cause of that problem?

The line below maintains the scrollTop of the Composer, which means it maintains the scroll position of the first line in the visible area. If the distance between the first line and the caret is greater than the Composer height after resizing, the first line is still visible, but the caret is not.

App/src/components/Composer/index.tsx

Line 269 in 526845b

textInput.current.scrollTop = prevScroll;

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

In Chrome, if the caret is not visible, after typing a charector, the caret will be scrolled to the visible area. We can implement similar behavior by replacing the above line with the code below.

const selectionRect = window.getSelection()?.getRangeAt(0).getBoundingClientRect();
const textInputRect = textInput.current.getBoundingClientRect();

if (selectionRect && (selectionRect.top < textInputRect.top || selectionRect.bottom > textInputRect.bottom)) {
    textInput.current.scrollTop += selectionRect.top - textInputRect.top;
} else {
    // eslint-disable-next-line react-compiler/react-compiler
    textInput.current.scrollTop = prevScroll;
}

What alternative solutions did you explore? (Optional)

In Chrome, after you blur and focus the Composer, the caret will be scrolled into the visible area. We can emulate this behavior by inserting the code below after the line mentioned in the root cause.

const selectionRect = window.getSelection()?.getRangeAt(0).getBoundingClientRect();
const textInputRect = textInput.current.getBoundingClientRect();

if (selectionRect && (selectionRect.top < textInputRect.top || selectionRect.bottom > textInputRect.bottom)) {
    textInput.current.blur();
    textInput.current.focus();
}

[QichenZhu]

Could we clarify the expected behavior? Since some content has to be hidden after shrinking the Composer, what should be visible after resizing? The first line, the middle of the text, the last line, or the caret ▎?

Behavior	Expanded	Collapsed
The first line is visible (current behavior)	Line 1 Line 2 Line 3 Line 4 ▎ Line 5 Line 6 Line 7 Line 8 Line 9 Line 10 Line 11	Line 1 Line 2 Line 3
The middle of the text is visible	Line 1 Line 2 Line 3 Line 4 ▎ Line 5 Line 6 Line 7 Line 8 Line 9 Line 10 Line 11	Line 5 Line 6 Line 7
The last line is visible	Line 1 Line 2 Line 3 Line 4 ▎ Line 5 Line 6 Line 7 Line 8 Line 9 Line 10 Line 11	Line 9 Line 10 Line 11
The caret is visible (my proposal)	Line 1 Line 2 Line 3 Line 4 ▎ Line 5 Line 6 Line 7 Line 8 Line 9 Line 10 Line 11	Line 4 ▎ Line 5 Line 6

[melvin-bot]

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

[rayane-djouah]

Could we clarify the expected behavior? Since some content has to be hidden after shrinking the Composer, what should be visible after resizing? The first line, the middle of the text, the last line, or the caret ▎?

Good question! @Expensify/design Could you clarify the expected result? The video below demonstrates the current behavior, where we keep the first visible line in view when resizing the composer. In my opinion, we should scroll to the caret position instead. What do you think?

Screen.Recording.2024-10-02.at.5.16.35.PM.mov

[dannymcclain]

Hmm, what's shown in the video honestly doesn't feel weird to me. It kinda looks like we're "respecting" the scroll position that was already there. But I also think it could make sense to always show the cursor/caret. So I think personally I'd opt for one of those two options. Let's see what @dubielzyk-expensify thinks!

[FitseTLT]

In my opinion, we should scroll to the caret position instead.

@rayane-djouah If a user has scrolled up from where the cursor is located and switches the composer size I doubt if the user expects it to be scrolled to the caret position. What I think and based my proposal on was we should as much as possible make the visible content of the composer consistent across the two composer size states. Currently, we are making the top content consistent on toggling composer size that's why we facing the current issue. So we can make the bottom or middle of content consistent instead. @dannymcclain WDYT

[dubielzyk-expensify]

I actually think we should respect the scroll position over the caret here. Cause I think you'd expect the view to expand not for the view to shift and follow the caret. I vote scroll position 👍

[dannymcclain]

Ok thanks @dubielzyk-expensify! That makes me think that our current behavior is what we want (keep the first visible line in view when resizing). Do you agree?

[FitseTLT]

Ok thanks @dubielzyk-expensify! That makes me think that our current behavior is what we want (keep the first visible line in view when resizing). Do you agree?

The logic of the expected result in the OP is that switching to full-size and back to normal-size, they expected the content visible to be on the same scroll level it was before toggling to full-size. This happens in most cases but when the scroll level is the down-end, when the user changes to full-size, the top line when it was non-full sized will now be in the middle on full-size (obviously b/c full-size will accomodate more content) so when swiching back to non-full sized it will keep the first line in view so it will be in a scrolled-up state than it was originally, which kind of unexpected.

[rayane-djouah]

PR under review

[melvin-bot]

Triggered auto assignment to @JmillsExpensify (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.

[anmurali]

Rotating the label so we can assign another BZ member while I am OOO

[melvin-bot]

@JmillsExpensify, @marcaaron, @FitseTLT, @rayane-djouah Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@JmillsExpensify, @marcaaron, @FitseTLT, @rayane-djouah Huh... This is 4 days overdue. Who can take care of this?

[rayane-djouah]

PR in review

[rayane-djouah]

PR approved

[melvin-bot]

@JmillsExpensify, @marcaaron, @FitseTLT, @rayane-djouah Eep! 4 days overdue now. Issues have feelings too...

[rayane-djouah]

PR under final review

[rayane-djouah]

PR is merged

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

• Fix - Chat - Compose box stays in the middle when reduced with a several lines message #50520

If no regressions arise, payment will be issued on 2024-12-16. 🎊

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

• @FitseTLT requires payment automatic offer (Contributor)
• @rayane-djouah requires payment automatic offer (Reviewer)

[melvin-bot]

@rayane-djouah @JmillsExpensify @rayane-djouah The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[rayane-djouah]

BugZero Checklist:

• Classify the bug:

  Bug classification

  Source of bug:

  • 1a. Result of the original design (eg. a case wasn't considered)
  • 1b. Mistake during implementation
  • 1c. Backend bug
  • 1z. Other:

  Where bug was reported:

  • 2a. Reported on production
  • 2b. Reported on staging (deploy blocker)
  • 2c. Reported on a PR
  • 2z. Other:

  Who reported the bug:

  • 3a. Expensify user
  • 3b. Expensify employee
  • 3c. Contributor
  • 3d. QA
  • 3z. Other:

• 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: https://github.com/Expensify/App/pull/38152/files#r1877946164

• If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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

• If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

• [@JmillsExpensify] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

#### Precondition:

- N/A

#### Test:

1. Open any chat.
2. In the compose box, start typing a message. Keep adding new lines until a scrollbar appears.
3. Tap on the "Expand" button to enlarge the compose box.
4. Continue adding lines to the message until a scrollbar appears again with the expanded compose box.
5. Reduce the size of the compose box.
6. Verify that the scroll position has followed the bottom of the compose bar.

Do we agree 👍 or 👎