[$2000] iOS mWeb - Scroll behavior on some pages is inconsistent and broken

[kavimuru]

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:

Scenarios:
1.Login with any account that has many reports.
2.From LHN, try to scroll to bottom with the first touch at the fab button, after that try to scroll normally at other position.
3.Notice you can't scroll right after that, you need to scroll 2,3 times to make it scroll as normal.

1.Select any workspace, choose Connect Bank Account => Connect Manually, press back button to Connect Bank Account, continue to press back.
2.Notice you can't scroll right after that, you need to scroll 2,3 times to make it scroll as normal.

1.Go to Settings => About page => Scroll to bottom, change the tab then go back.
2.Notice that you can’t scroll to bottom after that

Expected:

Can scroll to the bottom as normal

Actual:

Can not scroll to the bottom as normal

Workaround:

unknown

Platform:

Where is this issue occurring?

• Mobile Web Safari

Version Number: 1.2.39-0
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

RPReplay_Final1671032127.MP4

PBCQ6485.MP4

RPReplay_Final1671033212.MP4

RPReplay_Final1671120567.MP4

Expensify/Expensify Issue URL:
Issue reported by: @hungvu193
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1671032426349589

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~012729407713b5d561
• Upwork Job ID: 1606017543317778432
• Last Price Increase: 2022-12-29

[davidcardoza]

I was able to reproduce, sending out to external
https://user-images.githubusercontent.com/6643649/209216672-77a91412-be9d-4469-8d3d-30fa2d5b0ed7.MP4

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[dhanashree-sawant]

Proposal

Cause: IOS has feature of bounce on scroll whenever we reach extreme ends while scrolling. Whenever we scroll to bottom or top and content tries to bounce, on some occasion, due to the user trying to scroll beyond the content, it locks the parent View with it which changes touch focus to parent view and thus stopping the scroll.

Solution: add attribute bounces={false} in FlatList and ScrollView for that page which will stop the bounce on scroll feature and thus solve the issue. I will mention the code to change below for 'About' and 'Reports' page which are currently facing the issue. In future, if such issue occurs again on any other page, only thing we will need to add is the bounces attritube to the component

src/pages/settings/AboutPage/AboutPage.js
@@ -54,26 +54,27 @@
 <ScrollView
                contentContainerStyle={[
                    styles.flexGrow1,
                    styles.flexColumn,
                    styles.justifyContentBetween,
                ]}
 +             bounces={false}
            >

src/components/InvertedFlatList/BaseInvertedFlatList.js
@@ -121,26 +121,27 @@
            <FlatList
                // eslint-disable-next-line react/jsx-props-no-spreading
                {...this.props}
                ref={this.props.innerRef}
                renderItem={this.renderItem}
+              bounces={false}

[Santhosh-Sellavel]

I'm not able to reproduce this issue @kavimuru please check on the latest staging again!

RPReplay_Final1671815826.MP4

And personally, I think the issue might be caused to the banner as part of the view is not presented at the bottom.

[Santhosh-Sellavel]

@dhanashree-sawant

Thanks for the proposal, but we need the bounce effect!

cc: @pecanoro

[melvin-bot]

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

[Santhosh-Sellavel]

Waiting for proposals!

[pecanoro]

I can't reproduce anymore, but also I am using an emulator so I am not sure if it's different with a real device. @davidcardoza can you try again and see if you can reproduce it?

[hungvu193]

I'm still able to reproduce on latest version
https://user-images.githubusercontent.com/16502320/209892453-a88379c8-a117-4095-91ca-baf377e7a4d4.MOV

[melvin-bot]

📣 @shonsirsha You have been assigned to this job by @pecanoro!
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 📖

[shonsirsha]

Cool cool that sounds good, mostly just want to make sure all scrollable views/lists in the app will still scroll as expected!

I think it's a great solution, but yeah, we will have to be careful about testing so we don't break any other parts of the UI

Totally agree! Will keep testing stuff! 😄

Applied on Upwork, will create a PR today or latest tomorrow. Thanks!

[shonsirsha]

PR ready for review 🙂
cc @Santhosh-Sellavel @shawnborton

[melvin-bot]

@pecanoro, @davidcardoza, @shonsirsha, @Santhosh-Sellavel Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@pecanoro, @davidcardoza, @shonsirsha, @Santhosh-Sellavel Still overdue 6 days?! Let's take care of this!

[davidcardoza]

@pecanoro What is the next step on this one?

[pecanoro]

@davidcardoza Oh dammit, I think the issue wasn't linked properly on the PR and it didn't update the issue with the payment date. You can follow the steps here from step 6. You will need to pay the people involved since it's been more than a week since we deployed the changes. If you have any doubts about how to proceed, I recommend asking on the #bug-zero channel.

[0xmiros]

PR which fixes this issue causes serious regression in android chrome.
Pull-to-refresh not working.
Issue reported here: https://expensify.slack.com/archives/C049HHMV9SM/p1674040409891389

[shonsirsha]

EDIT:

It seems like the pull-to-refresh mentioned by @0xmiroslav also exists in newer iOS Safari as well as Chrome (iOS). Thus, my answer below to fix it won't actually fix anything. At this point, I don't think I could give a fix for this if we wanna keep the pull-to-refresh behaviour on mWeb, as p-t-r requires body to be scrollable.

We could simply revert the change as it's just a one-liner to get the pull-to-refresh functionality back, but the current issue with the iOS broken scrolling (this exact issue) will also appear back.

TLDR: if we want to have pull-to-refresh back on mWeb then we have to revert this, but iOS mWeb scrolling will be back to be a bit incosistent and broken (this issue #13599). Just a note, pull-to-refresh doesn't actually exist in the app, so if we wanna match how the app works we could leave it here.

OUTDATED ANSWER:

@0xmiroslav thanks for reporting. Although I did quite a thorough testing for this one, I missed it as I am unaware that the regression you mentioned is even a functionality in Android web. Sorry about this.

Anyhow, I don't find another way to solve this other than disabling the scrolling of the document's body. I'm now thinking of somehthing like this which is still the same but only does it in iOS Safari.

This should fix the regression & still fixes the issue:

File: index.html

<script>
function disableBodyScrollIOS(){
      if (isMobileSafari()){
            document.body.style.overflow = 'hidden';
      }
}
</script>

<body onload="disableBodyScrollIOS();">
etc...

isMobileSafari is a function that we have in the codebase that we can copy over to index.html. This is probably the safest approach as it doesn't touch anything other than iOS's Safari document's body styling.

Open for suggestions.
cc @pecanoro @Santhosh-Sellavel

Also would anyone lmk how to proceed if a PR causes regression? Thx.

[Santhosh-Sellavel]

PR which fixes this issue and causes serious regression in android chrome. Pull-to-refresh not working. Issue reported here: https://expensify.slack.com/archives/C049HHMV9SM/p1674040409891389

Good catch @0xmiroslav. But I think it is not a serious regression as it does not affect anything within our app. I will wait for @pecanoro to decide on this one.

Whether we fix it or not I think this is something that is a bit out of scope to find out in the PR review testing, thanks!

[pecanoro]

If pull to refresh does not seem to be a thing either on native apps, I would say this is not a bug and we can keep it like this on the mobile browsers to actually keep it consistent.

[Santhosh-Sellavel]

Thanks, everyone, @davidcardoza this is due for payment!

[davidcardoza]

Confirming payments with @pecanoro before sending out.

[davidcardoza]

• Payment sent to @shonsirsha
• @hungvu193 I hired you on the job, please accept to receive $250 payment
• @Santhosh-Sellavel please apply for the job so I can process payment - https://www.upwork.com/jobs/~012729407713b5d561

[Santhosh-Sellavel]

@davidcardoza Applied!

[Santhosh-Sellavel]

@davidcardoza bump!

[mallenexpensify]

Hired @Santhosh-Sellavel can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~012729407713b5d561

[Santhosh-Sellavel]

Thanks, @mallenexpensify done!

[mallenexpensify]

Paid @Santhosh-Sellavel

@davidcardoza and @Santhosh-Sellavel , can you fill out the below? The checklist didn't auto-populate

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:

• The PR that introduced the bug has been identified. Link to the PR: [$2000] iOS mWeb - Scroll behavior on some pages is inconsistent and broken #13599 (comment)
• 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: [$2000] iOS mWeb - Scroll behavior on some pages is inconsistent and broken #13599 (comment)
• 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.
• A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:

[Santhosh-Sellavel]

@pecanoro I believe this is not a regression, it existed right from the start!

[davidcardoza]

Thanks for taking over while i was away @mallenexpensify I filled out the two first bullet points.

[pecanoro]

@davidcardoza The bug-zero assignee usually takes care of the last bullet point, it would be great if you can take care of that last item, thank you!

[MelvinBot]

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

[davidcardoza]

This doesn't require a regression test it doesn’t make sense to add a step to TestRail test case.

[mallenexpensify]

I added it to the testing guidelines sheet https://docs.google.com/document/d/1UaWlcieiwLnsKfeKw7iwLF7vgYRmdw_-K8OcY5aPBRU/edit#heading=h.zm638pdvzx2