[$500] Web - Scroll speed has dramatically reduced in NewDot

[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!

---

Version Number: 1.3.99.0
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: @quinthar
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1699742883368309

Action Performed:

1. Go to staging.new.expensify.com
2. Log in with any account
3. Open any chat conversation
4. Scroll the content using two-finger drag

Expected Result:

Scroll speed should be the same as in Slack

Actual Result:

Scroll speed has dramatically reduced in NewDot

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

Recording.5433.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01df590e04409cd112
• Upwork Job ID: 1724876585592717312
• Last Price Increase: 2023-11-15

[melvin-bot]

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

[melvin-bot]

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

[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 - @rushatgabhane (External)

[kakajann]

Just to make sure... Do you want a momentum scroll?

[melvin-bot]

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

[kakajann]

Contributor details
Your Expensify account email: kakajanxxx@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0170cd9e3e2cc5613d

[melvin-bot]

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

[ZhenjaHorbach]

Proposal

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

Web - Scroll speed has dramatically reduced in NewDot

What is the root cause of that problem?

The main problem is using the inverted property for FlatList and a new patch for react-native-web after upgrading react-native-web

https://github.com/Expensify/App/blob/main/patches/react-native-web%2B0.19.9%2B002%2Bfix-mvcp.patch#L89

In this line, we halve the scrolling speed for inverted lists which greatly affects speed

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

I came to the conclusion that reducing the speed is a necessary measure
Because the scrolling speed is too fast by default when it's an inverted list
But the selected value(0.5) is too small

And instead of 0.5 we can increase this value to 0.7(or another value, but more testing is needed here to choose the ideal speed but as for me 0.7 looks optimal ) to make the list a little faster

Screen.Recording.2023-11-20.at.13.07.14.mov

What alternative solutions did you explore? (Optional)

Alternatively we may not use inverted property
Instead, we have an alternative way of using column-reverse
Which after tests looks much faster

For this, we need to update BaseInvertedFlatList like

const listStyle= isWeb
		? { flexDirection: 'column-reverse' }
		: undefined;

const contentContainerStyle = isWeb
		? { flexDirection: 'column-reverse' }
		: undefined;

const BaseInvertedFlatList = forwardRef((props, ref) => (
    <FlatList
        // eslint-disable-next-line react/jsx-props-no-spreading
        {...props}
        ref={ref}
        windowSize={15}
        maintainVisibleContentPosition={{
            minIndexForVisible: 0,
            autoscrollToTopThreshold: AUTOSCROLL_TO_TOP_THRESHOLD,
        }}
	inverted={!isWeb} // <-- do not invert to prevent transform: [{scaleY: -1}]
	contentContainerStyle={[props.contentContainerStyle, contentContainerStyle]} // <-- 'column-reverse' to make it scroll to the bottom
	style={[props.style, listStyle]} // <-- 'column-reverse' to reverse list array
    />
));

[rushatgabhane]

After research, I came to the conclusion that the main problem is using the inverted property for FlatList
Apparently due to the fact that our elements are inverted, this affects the scrolling speed of the list

@ZhenjaHorbach the inverted prop was there from the beginning. However this bug is very recent. So the suggested root cause shouldn't be correct.
Could you please dive deeper into the root cause and explain it in detail.

[ZhenjaHorbach]

After research, I came to the conclusion that the main problem is using the inverted property for FlatList

Apparently due to the fact that our elements are inverted, this affects the scrolling speed of the list

@ZhenjaHorbach the inverted prop was there from the beginning. However this bug is very recent. So the suggested root cause shouldn't be correct.

Could you please dive deeper into the root cause and explain it in detail.

No problem )

I'll look into this issue in more detail today )

[kaushiktd]

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

Web - Scroll speed has dramatically reduced in NewDot

What is the root cause of that problem?

Optimize the handleScroll function within the InvertedFlatList component by simplifying the timeout and event handling mechanisms

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

filePath:- https://github.com/Expensify/App/blob/main/src/components/InvertedFlatList/index.js#L82
Removed the condition that checks for lastScrollEvent.current and the associated timestamp comparison. This simplification avoids unnecessary complexity by directly setting the scrollEndTimeout whenever a scroll event occurs.
Consolidated the timeout logic to call onScrollEnd directly after the specified duration (250 milliseconds in this example). Adjust the timeout duration as needed based on your requirements for detecting the end of scrolling.

const handleScroll = (event) => {
  onScroll(event);

  if (scrollEndTimeout.current) {
    clearTimeout(scrollEndTimeout.current);
  }

  scrollEndTimeout.current = setTimeout(() => {
    onScrollEnd();
  }, 250); // Adjust the timeout duration if needed
};

Video:-

https://drive.google.com/file/d/1wPzTbIqzeAXdUsMXc6vrUOAwvtCs_5nu/view?usp=sharing

[ZhenjaHorbach]

@rushatgabhane

I have updated the proposition
#31402 (comment)
The problem is related to the update of react-native-web and the new patch that we use for this library

[rushatgabhane]

@ZhenjaHorbach's proposal looks good!

PR that caused the issue - #24482
Line that caused the issue - https://github.com/Expensify/App/blob/main/patches/react-native-web%2B0.19.9%2B002%2Bfix-mvcp.patch#L89

C+ reviewed 🎀 👀 🎀

[melvin-bot]

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

[rushatgabhane]

@joelbettner how do we proceed? is it as simple as removing 0.5 from this line?

https://github.com/Expensify/App/blob/main/patches/react-native-web%2B0.19.9%2B002%2Bfix-mvcp.patch#L89

[getusha]

@rushatgabhane could you check my comment here? #24482 (comment)

[JmillsExpensify]

Still discussing approach

[melvin-bot]

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

[JmillsExpensify]

@joelbettner mind weighing in so that we can move to the PR stage?

[melvin-bot]

@JmillsExpensify @joelbettner @rushatgabhane 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]

@JmillsExpensify, @joelbettner, @rushatgabhane Huh... This is 4 days overdue. Who can take care of this?

[JmillsExpensify]

@joelbettner bump on that last question? I'm also going to reach out to you over chat to make sure you've seen this.

[joelbettner]

Hey all...

I'm actually not so sure that this is a bug or something we want to fix. If we don't have to change the code from the react native patch...I think we should leave that alone.

The scroll speed seems normal to me. I tested this and did a side-by-side comparison with the scroll speed on the desktop app and didn't notice a difference. @JmillsExpensify do you mind testing it as well and letting me know what you think? I'm leaning towards just closing this issue.

[melvin-bot]

@JmillsExpensify @joelbettner @rushatgabhane 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!

[JmillsExpensify]

Yes, sure thing. Giving it a go now.

[melvin-bot]

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

[joelbettner]

Hearing no objections, I'm going to close this issue. Reopen it if you disagree.