Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android - Distance - Scrolling doesn't work when the number of waypoints increases #27382

Closed
1 of 6 tasks
lanitochka17 opened this issue Sep 13, 2023 · 8 comments
Closed
1 of 6 tasks
Assignees

Comments

@lanitochka17
Copy link

lanitochka17 commented Sep 13, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Issue found when executing PR #27076

Action Performed:

  1. Login into New Expensify
  2. Open Request Money from FAB
  3. Choose the Distance tab
  4. Add multiple new waypoints by selecting the "Add stop" button
  5. Attempt to scroll up

Expected Result:

User is able to scroll up to input stops

Actual Result:

User is unbale to scroll up

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.69-0

Reproducible in staging?: Yes

Reproducible in production?: No

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

Bug6199317_27076_Android_.mp4

Not reproducible in Production

Screen_Recording_20230914_003820_New.Expensify.mp4

Expensify/Expensify Issue URL:

Issue reported by: @situchan

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694021625336179

View all open jobs on GitHub

@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Sep 13, 2023
@OSBotify
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@melvin-bot
Copy link

melvin-bot bot commented Sep 13, 2023

Triggered auto assignment to @NikkiWines (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@NikkiWines
Copy link
Contributor

NikkiWines commented Sep 13, 2023

Yeah looks like it's almost certainly tied to #27076 since that modified scrolling for multiple waypoints.

cc: @jjcoffee @arosiclair

@NikkiWines
Copy link
Contributor

@arosiclair was able to reproduce this on v1.3.68-17 (yesterday's deploy) so this isn't actually a blocker and #27076 isn't the root cause.

This isn't a blocker, but will continue to look into to see if we can find the root cause for it.

@NikkiWines NikkiWines removed the DeployBlockerCash This issue or pull request should block deployment label Sep 13, 2023
@NikkiWines NikkiWines added Daily KSv2 and removed Hourly KSv2 labels Sep 13, 2023
@bernhardoj
Copy link
Contributor

bernhardoj commented Sep 14, 2023

Proposal

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

Can't scroll the waypoints list on Android only.

What is the root cause of that problem?

In the distance page, we have a nested ScrollView. One for the whole page, and one for the waypoint list. On Android, we are unable to scroll the nested ScrollView. This can be reproduced in a fresh new react native project.

This nested scrolling issue doesn't happen on iOS as nested scrolling is supported by default.

This issue starts to happen after we wrap the whole page with a ScrollView

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

To support nested scrolling on Android too, set nestedScrollEnabled (Android only prop) to true to the waypoints ScrollView.

<ScrollView
onContentSizeChange={(width, height) => {
if (scrollContentHeight < height && numberOfWaypoints > numberOfPreviousWaypoints) {
scrollViewRef.current.scrollToEnd({animated: true});
}
setScrollContentHeight(height);
}}
onScroll={updateGradientVisibility}
scrollEventThrottle={variables.distanceScrollEventThrottle}
ref={scrollViewRef}

@situchan
Copy link
Contributor

@bernhardoj's root cause is correct. It's scrolling limit in android.
nestedScrollEnabled=true also seems the only solution unless we prevent scrolling in parent view.

@NikkiWines
Copy link
Contributor

NikkiWines commented Sep 15, 2023

Ah cool, thank you for the investigation and determining that it's a regression of #26728.

@hayata-suenaga tagging you since you're the internal eng/ for that PR, do you want to handle this as a regression or shall we handle this as a separate issue and start looking at proposals?

@hayata-suenaga
Copy link
Contributor

thank you for letting me know about this issue Nikki

We can actually close this issue as we're making an UI change here and making the waypoint list not scrollable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants