[$500] Chat – Edited text is out of the frame

[kbecciv]

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: 1.4.11-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. Go to https://staging.new.expensify.com/
2. Log in with any Gmail account.
3. Navigate to any chat conversation.
4. Enter several lines of text until a scroller is visible
5. Send message
6. Click on edit this message
7. Wright more text using letters q, g, y

Expected Result:

Edited text is not out of the frame

Actual Result:

Edited text is out of the frame

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

Bug6309352_1702315424559.Edited_text_is_out_of_the_frame.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0114350bc651a8e598
• Upwork Job ID: 1734270946190327808
• Last Price Increase: 2023-12-25

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @laurenreidexpensify (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 Contributor-plus team member for initial proposal review - @cubuspl42 (External)

[ZhenjaHorbach]

Proposal

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

Chat – Edited text is out of the frame

What is the root cause of that problem?

The main problem is that when we add a new line, the padding we use above and below the input is ignored

App/src/styles/utils/index.ts

Lines 897 to 913 in d8f3268

	function getComposeTextAreaPadding(numberOfLines: number, isComposerFullSize: boolean): ViewStyle {
	let paddingValue = 5;
	// Issue #26222: If isComposerFullSize paddingValue will always be 5 to prevent padding jumps when adding multiple lines.
	if (!isComposerFullSize) {
	if (numberOfLines === 1) {
	paddingValue = 9;
	}
	// In case numberOfLines = 3, there will be a Expand Icon appearing at the top left, so it has to be recalculated so that the textArea can be full height
	else if (numberOfLines === 3) {
	paddingValue = 8;
	}
	}
	return {
	paddingTop: paddingValue,
	paddingBottom: paddingValue,
	};
	}

As a result, scroll is focused at the bottom of the text
But since some letters are higher than others
The bottom part is next to the border

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

I think that in this case we should not use padding, and instead we can use margin, so that the indentation we use at the bottom is more obvious
It is worth clarifying that the green border used in the input is not between padding and margin
This is part of the input wrapper

So we can update this function like (I kept the padding as it is part of other fixes (But more testing is needed here to make sure that the margin is suitable for previous fixes)) :

function getComposeTextAreaPadding(numberOfLines: number, isComposerFullSize: boolean): ViewStyle {
    // Issue #26222: If isComposerFullSize paddingValue will always be 5 to prevent padding jumps when adding multiple lines.
    if (!isComposerFullSize) {
        if (numberOfLines === 1) {
            return {
                paddingTop: 9,
                paddingBottom: 9,
            };
        }
        // In case numberOfLines = 3, there will be a Expand Icon appearing at the top left, so it has to be recalculated so that the textArea can be full height
        if (numberOfLines === 3) {
            return {
                paddingTop: 8,
                paddingBottom: 8,
            };
        }
    }
    return {
        marginTop: 5,
        marginBottom: 5,
    };
}

Screen.Recording.2023-12-11.at.20.03.44.mov

What alternative solutions did you explore? (Optional)

We can update this part of the code to remove the influence of padding on scrolling

    return {
        paddingTop: paddingValue,
        paddingBottom: paddingValue,
        scrollPaddingTop: paddingValue,
        scrollPaddingBottom: paddingValue,
    };

App/src/styles/utils/index.ts

Lines 909 to 912 in d8f3268

	return {
	paddingTop: paddingValue,
	paddingBottom: paddingValue,
	};

[botosdavid]

Proposal

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

The problem is that when we write new lines the padding on the bottom is no longer visible, the text "exits" its frame.

What is the root cause of that problem?

So if we pay close attention, in the first few lines this problem is not happening, but after we add multiple lines so that the input has a scrollbar, we can see that we are not scrolled fully to the bottom of the input. We are just in the vision of the cursor, but we can scroll down a couple of pixels.

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

We just need to scroll all the way down whenever we are writing on the end of the text (when our cursor is at the end).

Screen.Recording.2023-12-11.at.22.29.35.mov

What alternative solutions did you explore? (Optional)

Alternatively we can check if we are on the last line of the input (not just at the last character).

  useEffect(() => {
        if (textInput.current) {
            const cursorPosition = textInput.current.selectionStart;
            // Scroll to the bottom only if on the last position
            if (cursorPosition === value.length) {
                textInput.current.scrollTop = textInput.current.scrollHeight;
            }
        }
      }, [value]);

[melvin-bot]

📣 @botosdavid! 📣
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>

[botosdavid]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01d08bd0c2eb123bd2

[melvin-bot]

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

[prafullvyas272]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~0114930272380f2d09

[melvin-bot]

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

[laurenreidexpensify]

@cubuspl42 bump for review ^^ thanks

[cubuspl42]

@ZhenjaHorbach

But more testing is needed here to make sure that the margin is suitable for previous fixes

Unfortunately, this testing should be a part of a work on a proposal. You correctly observed that the code is crafted to solve a different issue, and it's especially easy to introduce a regression in such cases. We have to be confident that the proposal both solves our current issue and doesn't introduce regressions.

@botosdavid

Are you suggesting any code changes in your "What changes do you think we should make in order to solve the problem?" section?

@dragnoir

Have you tested this change on Chrome and Safari, both desktop and mobile?

[ZhenjaHorbach]

@ZhenjaHorbach

But more testing is needed here to make sure that the margin is suitable for previous fixes

Unfortunately, this testing should be a part of a work on a proposal. You correctly observed that the code is crafted to solve a different issue, and it's especially easy to introduce a regression in such cases. We have to be confident that the proposal both solves our current issue and doesn't introduce regressions.

@botosdavid

Are you suggesting any code changes in your "What changes do you think we should make in order to solve the problem?" section?

@dragnoir

Have you tested this change on Chrome and Safari, both desktop and mobile?

Therefore, in the proposal I did not touch this part of the code
Moreover, we don’t need to change those values
Since those changes are needed only for textInput in which there is no scroll
In our case, we have a bug related to scrolling

And I add an alternative proposal which will work too

[botosdavid]

@ZhenjaHorbach

But more testing is needed here to make sure that the margin is suitable for previous fixes

Unfortunately, this testing should be a part of a work on a proposal. You correctly observed that the code is crafted to solve a different issue, and it's especially easy to introduce a regression in such cases. We have to be confident that the proposal both solves our current issue and doesn't introduce regressions.

@botosdavid

Are you suggesting any code changes in your "What changes do you think we should make in order to solve the problem?" section?

@dragnoir

Have you tested this change on Chrome and Safari, both desktop and mobile?

Yes, I placed the code snippet in the alternative solutions section by mistake, but I was suggesting something like that to scroll down when needed, as the padding is there already, but only not visible because we are not scrolled down fully.

[dragnoir]

Testing on other platforms ASAP

[dragnoir]

The scroll-padding: 10px; is working only on Chrome.

I think this is not a bug, below the same in other websites using textarea.

==> Image from the video by kbecciv

==> screenshot from Mozilla web docs

==> screenshot from W3Schools

[melvin-bot]

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

[melvin-bot]

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

[laurenreidexpensify]

Waiting for proposals

[cubuspl42]

@ZhenjaHorbach @dragnoir Please update the proposals so they represent the best, most actionable ideas of yours

@ZhenjaHorbach I'm confused about whether you consider your primary or alternative ideas more preferable. It might be partially a linguistic issue. Sometimes I'm not sure what you mean.

@dragnoir So is your proposal now outdated, as it doesn't work on Safari?

[dragnoir]

@cubuspl42 if you can check the same flow on other websites, I can see the exact bahavior (near frame text). You can zoom the browser to see the small details. I assume it's not a bug.

Note: the video posted by the reporter, does not show out of the frame text, just no space between text and the frame.

[cubuspl42]

@dragnoir

I assume it's not a bug.

In the case of frontend Web apps, it's an everyday reality that some problems are considered a bugs even when they come from an issue in the browsers, a specific browser, or sometimes even the standard itself. Sometimes there are reasonable workarounds.

Still, in this case, the impact seems to be quite small and all proposals so far have some gotchas. So I would agree to close this one.

@laurenreidexpensify Would you agree?

[melvin-bot]

@cubuspl42 @laurenreidexpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

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

[melvin-bot]

@cubuspl42, @laurenreidexpensify Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@cubuspl42, @laurenreidexpensify Still overdue 6 days?! Let's take care of this!