[C+ Checklist Needs Completion] [$1000] DEV:Takes time to change topbar(manual to scan, scan to manual) selection in request money

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

1. click "+" button
2. select request money
3. try to slide right to left

   Expected Result:

   selection change should be immediately

   Actual Result:

   selection change takes some time

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

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

Version Number: Dev Reproducible in staging?: dev Reproducible in production?: dev 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://github.com/Expensify/App/assets/43996225/e06b81a2-2165-4a53-a742-80a0555108f2

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010c514d8b4722d88a
• Upwork Job ID: 1692452963606020096
• Last Price Increase: 2023-08-18

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Bug0 Triage Checklist (Main S/O)

• [x] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• [x] 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.
• [x] 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.
• [x] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[allroundexperts]

Proposal

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

Tab change not animating when screen is swiped when on MoneyRequestPage.

What is the root cause of that problem?

The root cause is that we're having static values as background colours for the buttons, icons and text inside the tab.

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

We need to make use of the position property here and pass that onto the TabSelector component. Inside it, we need to interpolate this position value between:

1. Background colour of the button in selected / un-selected state.
2. Text colour of the button in selected / un-selected state.
3. Icon colour of the button in selected / un-selected state.

These interpolated values of colours are then to be used inside TabSelectorItem component. This will result in a smooth transition which will dispel the lag effect.

What alternative solutions did you explore? (Optional)

We can also just disable the swipe on web so that the user can only switch the tab by pressing the button itself. However, I think that the solution proposed above is more elegant.

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~010c514d8b4722d88a

[melvin-bot[bot]]

Current assignee @greg-schroeder 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 - @burczu (External)

[greg-schroeder]

I can't repro DEV issues - I'm assuming since you made a proposal here @allroundexperts that you were able to?

[allroundexperts]

Hi @greg-schroeder!

Yes, this is easily reproducible.

@shawnborton How should the animation look like here? Do you think animating the colours as the user slides the screen makes sense?

[studentofcoding]

Proposal

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

Tab change animations are delayed when the screen is swiped when on MoneyRequestPage.

What is the root cause of that problem?

The root cause of this problem is that by default, React Navigation lazy loads tab screens, which means that the screens are only rendered when accessed for the first time. This can cause a delay in rendering the new tab screen when swiping.

When the screen is swiped, the new tab screen needs to be rendered immediately to provide a smooth transition. However, because of lazy loading, there is a delay in rendering the new tab screen, resulting in delayed animation.

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

What we need to do is simply disable lazy by setting the lazy prop of the OnyxTabNavigator component to false in MoneyRequestSelectorPage.js. This ensures that all tab screens are rendered upfront, eliminating the delay in the tab change animation.

<OnyxTabNavigator
                                    lazy={false} // Add this line
                                    id={CONST.TAB.RECEIPT_TAB_ID}
                                    tabBar={({state, navigation}) => (
                                        <TabSelector
                                            state={state}
                                            navigation={navigation}
                                            onTabPress={resetMoneyRequestInfo}
                                        />
                                    )}
                                >
                                    <TopTab.Screen
                                        name={CONST.TAB.MANUAL}
                                        component={NewRequestAmountPage}
                                        initialParams={{reportID, iouType}}
                                    />
                                    {canUseScanReceipts && (
                                        <TopTab.Screen
                                            name={CONST.TAB.SCAN}
                                            component={ReceiptSelector}
                                            initialParams={{reportID, iouType}}
                                        />
                                    )}
                                    {canUseDistanceRequests && (
                                        <TopTab.Screen
                                            name={CONST.TAB.DISTANCE}
                                            component={DistanceRequestPage}
                                        />
                                    )}
                                </OnyxTabNavigator>

What alternative solutions did you explore? (Optional)

None

cc @greg-schroeder

[shawnborton]

This is not a bug, we can close. cc @AndrewGable

[allroundexperts]

@shawnborton Is there any specific reason why this is not a bug? The tabs are clearly getting stuck.

[shawnborton]

Based on the video the tabs don't look stuck. Looks like they slide in and out as intended.

[allroundexperts]

@shawnborton Please watch from 8s onwards.

[shawnborton]

It's really hard to tell because the mouse is moving like crazy in that video. Can we create a new video where the mouse is doing normal gestures?

[allroundexperts]

Here you go @shawnborton.

https://github.com/Expensify/App/assets/30054992/93ca21fa-d718-44b9-92d3-0420a8e50a92

[shawnborton]

Cool, thanks for that. Curious for @AndrewGable's thoughts here.

[redpanda-bit]

Proposal

Selection change between tab screens is not immediate due to not using realtime animation of tab active state.

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

Make tabs active state have a faster feedback between screen changes.

What is the root cause of that problem?

It takes about a second for react-navigation/material-top-tabs to update navigation state with the current active tab, this time is reflected in this UI lag.

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

Instead of changing states based on isSelected use the position property to determine active state.

https://github.com/Expensify/App/assets/25206487/1da37484-001f-433d-99b4-361625bda269

What alternative solutions did you explore? (Optional)

I consider adding new animations a risk at the moment. There is an android crash in react native related to animations. If it is not urgent, this should wait for that bug to be fixed in react native first, otherwise fixing this could introduce a different bug.

[greg-schroeder]

going to wait on @AndrewGable's take before I take any action on this one like potentially closing or not

[greg-schroeder]

bump @AndrewGable

[AndrewGable]

Yes this is something that can be improved. I agree with it being a bug.

[burczu]

@studentofcoding I've checked your proposed solution and it does not work...

@allroundexperts and @carlosalmonte04 - could you please be more specific of what exactly should be changed and how? I feel your proposals may be similar (using position to make animation smooth).

[redpanda-bit]

Hey @burczu I borrowed the idea from @allroundexperts's proposal, I didn't realize how similar they were. His proposal should be accepted.

there are two options for the approach on the background color 1) a single element sliding around the tabs similar to this https://reactnavigation.org/docs/material-top-tab-navigator/ (this approach is a lot more changes). 2) background color changes like so https://github.com/Expensify/App/issues/25428#issuecomment-1684205294 (looks good but not the common material-tabs approach, IMO).

[burczu]

@carlosalmonte04 thanks for the explanation!

I agree we should choose the proposal from @allroundexperts - using the position property to change top bar button colors and background should do the trick

🎀 👀 🎀 C+ reviewed

[melvin-bot[bot]]

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

[allroundexperts]

Before moving forward, @shawnborton can you confirm what sort of animation is required here? My understanding is that animating the colours (button background / text colour) only would be sufficient. Let me know if you agree.

[shawnborton]

Nothing should be changing about the animation, that's not the issue at hand. We just need to make sure the tab styles actually fire as soon as the tab changes.

[allroundexperts]

Nothing should be changing about the animation, that's not the issue at hand. We just need to make sure the tab styles actually fire as soon as the tab changes.

The tab changes only once the swipe is complete. During the swipe, we need some sort of animation like below:

https://github.com/Expensify/App/assets/30054992/e63a3819-ac6f-435a-ad7f-e9a43c087c11

Notice how the blue line shifts when the user swipes. We need a similar animation for the intermediate state when the user is swiping the tabs.

[shawnborton]

we need some sort of animation like below

We don't actually need this. Let's just keep what we have by now and change the tab style once the new tab is fully into view.

[allroundexperts]

That is whats happening right now.

[shawnborton]

Okay, then let's just leave things as they are.

Or maybe this isn't actually a bug? Now that I see this again, are you purposely holding down your mouse click after you switch tabs?

[allroundexperts]

Or maybe this isn't actually a bug? Now that I see this again, are you purposely holding down your mouse click after you switch tabs?

I don't think so. Try swiping yourself on web chrome!

[shawnborton]

Okay cool, so let's just make sure the tab highlight changes as soon as the new tab is fully in view. No need to change anything with animation, etc. That's not what this issue is about.

[allroundexperts]

That is not what I proposed. My proposal suggests to add a animation to fix this. @burczu I guess we want more proposals for what Shawn is asking for above.

[burczu]

@allroundexperts I think what @shawnborton mean is to not add this additional animated bottom border etc. that is shown in the example you've posted here - I think fixing it with the position property to change button styles immediately instead waiting for the whole tab to be loaded is the correct way (please check the screen recording from @carlosalmonte04 proposal). Am I right @shawnborton?

[shawnborton]

That is exactly correct. Let's fix the tab highlighting first before adding any new features like animation.

[thienlnam]

@burczu Does the proposal from @allroundexperts fix the issue in the way Shawn is intending?

[allroundexperts]

@thienlnam Yes. It does. I mis-understood the animations part.

[thienlnam]

Cool - will assign ya then!

[melvin-bot[bot]]

📣 @allroundexperts Please request via NewDot manual requests for the Contributor role ($1000)

[allroundexperts]

PR created https://github.com/Expensify/App/pull/26022

[burczu]

@thienlnam I've approved this PR, but Bot did not assign any internal engineer automatically - could you help with it? Or maybe it is related to current code freeze?

[allroundexperts]

Created a follow up PR as I discovered that there were some prop type warnings on native devices.

[burczu]

Hey @allroundexperts! What prop types warnings do you get on native devices?

[allroundexperts]

@burczu Following warning appears:

https://github.com/Expensify/App/assets/30054992/92ad211b-5610-47b6-b8b0-c85601b57139

[burczu]

@allroundexperts - so shouldn't we accept numbers and objects instead of any? We will switch to TypeScript soon and I expect that any type will be forbidden anyway...

[allroundexperts]

Object is forbidden as well @burczu.

[allroundexperts]

Also, with types, things will be different as we won't be getting warnings on the compiled code.

[burczu]

Alright, let's go with any for now - I believe it will change, while doing migration to TS.

[allroundexperts]

Yep. With typescript number will work just fine (because it won't be platform dependent)

[melvin-bot[bot]]

Reviewing label has been removed, please complete the "BugZero Checklist".