[HOLD for payment 2023-12-26] [$1000] mWeb - Composer - Unnecessary blank area is created when scrolling down while keyboard is open

[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 testing link https://staging.new.expensify.com/
2. Open any chat
3. Write multiline text in the composer until the enlarge composer icon appear (keep the keyboard open)
4. Tap on the maximize composer icon
5. Above the keyboard, scroll down

Expected Result:

The bottom of the screen should stick to the bottom of the input composer, and no extra blank area should be created when the user scrolls down on the screen

Actual Result:

When scrolling down while the composer is maximized, an excessively wide area is created below the composer, resulting in continuous resizing of the composer when scrolling both upward and downward

Workaround:

Unknown

Platforms:

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

• [ ] Android / native
• [ ] Android / Chrome
• [ ] iOS / native
• [x] iOS / Safari
• [ ] MacOS / Chrome / Safari
• [ ] MacOS / Desktop

Version Number: 1.3.18.0

Reproducible in staging?: yes

Reproducible in production?: Yes - but the composer does not keep resizing, only the blank area is presented

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

https://platform.applause.com/services/links/v1/external/8e4abb883a4188a43758f0561fbfd5c7900892adaded42ecb9fb365ac7c0b0a3

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01302d5f678fb0c5f7
• Upwork Job ID: 1662142490327298048
• Last Price Increase: 2023-05-26
• Automatic offers:
• suneox | Contributor | 28039355

[melvin-bot[bot]]

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

[melvin-bot[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[bot]]

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

[melvin-bot[bot]]

Current assignee @abekkala is eligible for the External assigner, not assigning anyone new.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[mountiny]

We can export this to Callstack

[lukemorawski]

here I am

[melvin-bot[bot]]

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

[melvin-bot[bot]]

📣 @lukemorawski You have been assigned to this job by @mountiny! Please apply to this job in Upwork 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 📖

[qasimkhan23]

Proposal

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

When scrolling down while the composer is maximized, an excessively wide area is created below the composer, resulting in continuous resizing of the composer when scrolling both upward and downward.

What is the root cause of that problem?

We have added the marginTop property in screenWrapperStyle and we are providing the value to it from viewportOffsetTop which is causing the marginTop value to fluctuate when a user drag the screen in IOS Safari browser, and the View which contains the chat has given the property flex:1 because of that the Inner View adjusts its size and the composer looks resizing continuously. https://github.com/Expensify/App/blob/967100432a5f3ae5458f43ab20163ea07f6d17dc/src/pages/home/ReportScreen.js#L229

before

you can see below, how the top of the page got cut when I scroll the page down, when I scroll the page down the offset value is provided in a negative number which results in cutting the top part of the page because of marginTop: negativeValue

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

There is no need to add viewportOffsetTop because we have already provided flex:1 to wrapper which will take care of the screen on different devices. after applying the solution

  const screenWrapperStyle = [styles.appContent, styles.flex1];

What alternative solutions did you explore? (Optional)

None

[melvin-bot[bot]]

@abekkala, @thienlnam, @Santhosh-Sellavel, @lukemorawski Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[Santhosh-Sellavel]

This is picked by call stack waiting for @lukemorawski

[lukemorawski]

Working on a solution. Already have one but investigating a more comprehensive one.

[lukemorawski]

Still working on a more robust solution. Will have more info today.

[lukemorawski]

After some trial and error I can say that preventing scrolling of the whole window when the user activates the virtual keyboard is not achievable for current app UI setup. Common solutions and libraries (such as body-scroll-lock) don't seem to have an effect. The whole thing is caused by iOS Safari rather peculiar way of dealing with a problem of obscured input elements. To prevent any input element from being hidden under the keyboard, in an eventuality when there is not enough room under that element, Safari adds some additional space at the bottom of the window. The whole problem is that this space seems to be not a part of the DOM tree. It's not added body or html height or on any other node. It's just some space under the DOM tree. This means that it's beyond control of the available CSS or JS apis. The only way to mitigate this problem is to prevent the window element from scrolling, by applying some styles to body and preventing default behaviour of the scroll event (plus some other more finesse tricks). Unfortunately due to a very deep anchoring of the scrolled div and it's layout this doesn't work. The only way I have found is to scroll the window content back to it's optimal position after the user over scrolled it downwards. I have a working solution but this needs to be discussed internally and now I'm delegated to an another, more important issue, so the PR will be slightly delayed.

[Santhosh-Sellavel]

@lukemorawski

Can you follow the proposal template and leave a new comment

[Santhosh-Sellavel]

@lukemorawski Any update?

[lukemorawski]

@Santhosh-Sellavel Posted internally an updated proposal with a working solution. Waiting for acceptance.

[lukemorawski]

Updated Proposal

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

The problem we are trying to solve is the incorrect behavior of the input composer in the chat feature of the Expensify web application on iOS/Safari. When the composer is focused and the user scrolls down, an excessively wide area is created below the composer, resulting in continuous resizing of the composer when scrolling in both directions. This behavior is not aligned with the expected behavior, which is for the input composer to stick to the bottom of the screen without any extra blank area being created when scrolling. This behaviour occurs not only for a maximised composer but also for the compact one.

What is the root cause of that problem?

The whole thing is caused by iOS Safari rather peculiar way of dealing with a problem of obscured input elements. To prevent any input element from being hidden under the keyboard, in an eventuality when there is not enough room under that element, Safari adds some additional space at the bottom of the window. The whole problem is that this space seems to be not a part of the DOM tree. It's not added body or html height or on any other node. It's just some space under the DOM tree. This means that it's beyond control of the available CSS or JS apis. The DOM api registers viewPort and window height decrease after the keyboard shows up, yet in fact you can see that this space increases. The only way to mitigate this problem is to prevent the window element from scrolling, by applying some styles to body and preventing default behaviour of the scroll event (plus some other more finesse tricks). Unfortunately due to a very deep anchoring of the scrolled div and it's layout this doesn't work.

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

After some trial and error I can say that preventing scrolling of the whole window when the user activates the virtual keyboard is not achievable for current app UI setup. Common solutions and libraries (such as body-scroll-lock) don't seem to have an effect. The solution I have found to work is to scroll back the window content back to it's optimal position after the user stops scrolling (see attached video) This is can be done by creating a HOC that wraps the ReportScreen and checks if the keyboard is open, if so then scrolls the window back to it's desired position after the touchend event. This also requires adding a helper module that would extend RN Keyboard to use addEventListener on the web. Currently RNW exposes this method, but it does nothing. We could add a module that would simply reexport all the methods on native, while on the web, it would add this missing method using browser APIs (checking if the VP size changed, and then checking for active element and if it's an element that exposes a keyboard). Change to SwipeableView would be also desirable to make to onSwipeDown callback work on the web (currently only native). This would prevent unnecessary scrolling of the window while swiping on the composer div. This can be done by listening to the touchstart/touchend events and calculating the diff between the clientY values. When that diff is greater then some minimal value, the onSwipeDown callback is triggered.

What alternative solutions did you explore? (Optional)

Scroll blocking was explored but only seemed to work for elements directly inside the body element.

https://github.com/Expensify/App/assets/134388952/c67523c4-c6cc-4f26-92ff-43c692110770

Works also when the composer text area field is expanded

[melvin-bot[bot]]

@abekkala @thienlnam @Santhosh-Sellavel @lukemorawski 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!

[abekkala]

@Santhosh-Sellavel what do you think of the updated proposal above

[Santhosh-Sellavel]

Looking again seems this is something specific to mWeb iOS Safari. This is a known issue we decide to do nothing as we always get workarounds. What's proposed here is also a work around.

[lukemorawski]

@Santhosh-Sellavel Well maybe in this case it's worth having a workaround and leave the end user with an incrementally better UX, than just leave it as it is, because the proper solution is unachievable due to one browser buggy/odd way of dealing with a simple thing? This solution can be applied only in places where this issue exists, so it won't affect any other part of the app. Especially it won't apply to the native side, as it's limited only to web browsers. I could narrow it down even further, by applying it only to mobile safari.

[Santhosh-Sellavel]

Let's give it a try then, what do you say @thienlnam?

[thienlnam]

Hmm, is this something that would eventually be fixed within iOS Safari and wouldn't this apply to all cases where you're able to scroll and the keyboard is open on iOS?

We've definitely added platform specific code to handle certain cases and then it only caused us headache in the future having to deal with other edge cases that ended up popping up from those changes. This kind of feels like the same scenario that might happen.

I'm more curious if there is an open bug report for this on iOS and if we can apply pressure to have it fixed there

[lukemorawski]

@thienlnam It is reported as a bug here: https://bugs.webkit.org/show_bug.cgi?id=141832 Yet, looks like authors treat it more like a feature :) It was reported in 2015 and till this day it wasn't resolved, and probably won't be. There might be some hope in new dvh view port height unit, but it's only supported in the very latest browsers and it needs to be tested, specifically in mobile safari. WebKit might still add this extra space under the vp, no new length unit won't help.

[thienlnam]

Ah yeah yikes - this isn't ideal. Shall we try to this change and see how it holds up?

[lukemorawski]

@thienlnam I would certainly give it a try. It's easy to remove in the future by simply deleting a HOC from the list in compose.

[melvin-bot[bot]]

@abekkala @thienlnam @Santhosh-Sellavel @lukemorawski this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[lukemorawski]

@thienlnam @Santhosh-Sellavel any news on that one?

[Santhosh-Sellavel]

Yeah, let's give it a try. Please work on it.

[lukemorawski]

Great!

[melvin-bot[bot]]

@abekkala, @thienlnam, @Santhosh-Sellavel, @lukemorawski Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot[bot]]

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

[abekkala]

@JmillsExpensify Reassigning for Sabbatical

---

CURRENT STATUS:

This will be heading to payment stage soon.

• Issue reported by: Applause - Internal Team
• Contributor selected for fix: @lukemorawski (Callstack)
• C+ Review: @Santhosh-Sellavel

[melvin-bot[bot]]

@JmillsExpensify, @thienlnam, @Santhosh-Sellavel, @lukemorawski Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

@JmillsExpensify, @thienlnam, @Santhosh-Sellavel, @lukemorawski Still overdue 6 days?! Let's take care of this!

[melvin-bot[bot]]

@JmillsExpensify, @thienlnam, @Santhosh-Sellavel, @lukemorawski 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

[melvin-bot[bot]]

@JmillsExpensify, @thienlnam, @Santhosh-Sellavel, @lukemorawski 12 days overdue. Walking. Toward. The. Light...

[melvin-bot[bot]]

This issue has not been updated in over 14 days. @JmillsExpensify, @thienlnam, @Santhosh-Sellavel, @lukemorawski eroding to Weekly issue.

[thienlnam]

Issue is in the reviewing stage - Luke has been out for a bit and just recently got back so we're still chugging along

[JmillsExpensify]

Thanks for updating!

[thienlnam]

After some recent changes in main, there is some unexpected functionality that is blocking the PR https://github.com/Expensify/App/pull/21298#issuecomment-1680572874

We're going to hold this ticket, and create another ticket for this new bug that is on main

[thienlnam]

Cc @lukemorawski Could you summarize what the bug is and steps to reproduce so we can get a bug created?

EDIT: Actually, you should have access to the #expensify-bugs channel. Could you create a bug report there and then tag me on that bug report so we can get an issue created?

[melvin-bot[bot]]

This issue has not been updated in over 15 days. @JmillsExpensify, @thienlnam, @Santhosh-Sellavel, @lukemorawski eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[JmillsExpensify]

What's the latest on this issue?

[thienlnam]

Looks like the bug was fixed in ios17, and cannot be reproduced - I believe we should be able to merge main and continue in this issue then. @lukemorawski do you also agree?

[JmillsExpensify]

Sounds great to me! Let's get this one wrapped up.

[lukemorawski]

@thienlnam @JmillsExpensify yep, sounds good