Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.32k stars 2.75k forks source link

[$125] Web - IOU - Swiping from distance tab brings the waypoint selector to the scan tab #47653

Closed lanitochka17 closed 1 week ago

lanitochka17 commented 3 weeks ago

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: 9.0.22-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: N/A Email or phone of affected tester (no customers): nathanmulugetatesting+1030@gmail.com Issue reported by: Applause - Internal Team

Action Performed:

  1. Navigate to staging.new.expensify.com
  2. Click on FAB > Submit expense
  3. Click on the distance tab
  4. Hover over the "Start" waypoint and swipe to the left to go to the scan tab

Expected Result:

The tab changes without any issues and the waypoint selector doesn't appear on the scan tab

Actual Result:

The waypoint selector moves to the scan tab for a few seconds after a swipe

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/328a617a-c403-4d8c-b3bb-0531e272d445

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01ebd6f4ddf963d9c7
  • Upwork Job ID: 1826759942640304550
  • Last Price Increase: 2024-08-29
Issue OwnerCurrent Issue Owner: @ikevin127
melvin-bot[bot] commented 3 weeks ago

Triggered auto assignment to @twisterdotcom (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

lanitochka17 commented 3 weeks ago

@twisterdotcom FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

lanitochka17 commented 3 weeks ago

We think that this bug might be related to #vip-vsp

nkdengineer commented 3 weeks ago

Proposal

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

The waypoint selector moves to the scan tab for a few seconds after a swipe

What is the root cause of that problem?

When we drag the waypoint we don't disable the swipe option of the tab navigator then the weird behavior happens

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

  1. OnyxTabNavigator should accept a new prop screenOptions and DraggableList accept a new prop onDragStart

  2. In IOURequestStartPage pass screenOptions with swipeEnabled to OnyxTabNavigator here

const [swipeEnabled, setSwipeEnabled] = useState(true);
screenOptions={{
    swipeEnabled,
}}

https://github.com/Expensify/App/blob/80236f5bd80086950344eb10aa89c174bf7c923e/src/pages/iou/request/IOURequestStartPage.tsx#L135

  1. Pass setSwipeEnabled to IOURequestStepDistance

https://github.com/Expensify/App/blob/80236f5bd80086950344eb10aa89c174bf7c923e/src/pages/iou/request/IOURequestStartPage.tsx#L163

  1. When the drag starts we will update swipeEnabled to false and update it to true when the drag ends.
onDragEnd={(params) => {
    updateWaypoints(params);
    setSwipeEnalbed?.(true);
}}
onDragStart={() => {
    setSwipeEnalbed?.(false);
}}

https://github.com/Expensify/App/blob/80236f5bd80086950344eb10aa89c174bf7c923e/src/pages/iou/request/step/IOURequestStepDistance.tsx#L484

What alternative solutions did you explore? (Optional)

Simple pass screenOptions to OnyxTabNavigator with swipeEnabled as false if the selectedTab is the distance tab

screenOptions={{
    swipeEnabled: selectedTab !== CONST.TAB_REQUEST.DISTANCE,
}}

https://github.com/Expensify/App/blob/80236f5bd80086950344eb10aa89c174bf7c923e/src/pages/iou/request/IOURequestStartPage.tsx#L135

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

twisterdotcom commented 2 weeks ago

Niche, so downgrading.

melvin-bot[bot] commented 2 weeks ago

Upwork job price has been updated to $125

ikevin127 commented 2 weeks ago

@nkdengineer Thank you for the proposal. Unfortunately your main solution does not work in fixing the issue, check out the video below and my changes diff and let me know in case I did not apply the solution correctly.

https://github.com/user-attachments/assets/32742ca7-4a96-4aa4-847c-c13d7881b0fb

DIFF ```diff diff --git a/src/components/DraggableList/index.tsx b/src/components/DraggableList/index.tsx index 418f3e93eac..a98689d575a 100644 --- a/src/components/DraggableList/index.tsx +++ b/src/components/DraggableList/index.tsx @@ -38,6 +38,7 @@ function DraggableList( shouldUsePortal = false, // eslint-disable-next-line @typescript-eslint/naming-convention ListFooterComponent, + onDragStart, }: DraggableListProps, ref: React.ForwardedRef, ) { @@ -79,7 +80,10 @@ function DraggableList( style={styles.flex1} contentContainerStyle={styles.flex1} > - + {children} diff --git a/src/pages/iou/request/IOURequestStartPage.tsx b/src/pages/iou/request/IOURequestStartPage.tsx index eac30b8839d..b83675259ec 100644 --- a/src/pages/iou/request/IOURequestStartPage.tsx +++ b/src/pages/iou/request/IOURequestStartPage.tsx @@ -38,6 +38,7 @@ function IOURequestStartPage({ }: IOURequestStartPageProps) { const styles = useThemeStyles(); const {translate} = useLocalize(); + const [swipeEnabled, setSwipeEnabled] = useState(true); const [isDraggingOver, setIsDraggingOver] = useState(false); const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`); const policy = usePolicy(report?.policyID); @@ -93,6 +94,8 @@ function IOURequestStartPage({ return [headerWithBackBtnContainerElement, tabBarContainerElement, activeTabContainerElement].filter((element) => !!element) as HTMLElement[]; }, [headerWithBackBtnContainerElement, tabBarContainerElement, activeTabContainerElement]); + console.log('swipeEnabled', swipeEnabled); + if (!transaction?.transactionID) { // The draft transaction is initialized only after the component is mounted, // which will lead to briefly displaying the Not Found page without this loader. @@ -139,6 +142,9 @@ function IOURequestStartPage({ tabBar={TabSelector} onTabBarFocusTrapContainerElementChanged={setTabBarContainerElement} onActiveTabFocusTrapContainerElementChanged={setActiveTabContainerElement} + screenOptions={{ + swipeEnabled, + }} > {() => ( @@ -161,7 +167,10 @@ function IOURequestStartPage({ {() => ( - + )} diff --git a/src/pages/iou/request/step/IOURequestStepDistance.tsx b/src/pages/iou/request/step/IOURequestStepDistance.tsx index 1798a95490a..32e1c7dbfa7 100644 --- a/src/pages/iou/request/step/IOURequestStepDistance.tsx +++ b/src/pages/iou/request/step/IOURequestStepDistance.tsx @@ -71,6 +71,7 @@ function IOURequestStepDistance({ personalDetails, currentUserPersonalDetails, skipConfirmation, + setSwipeEnabled, }: IOURequestStepDistanceProps) { const styles = useThemeStyles(); const {isOffline} = useNetwork(); @@ -481,7 +482,13 @@ function IOURequestStepDistance({ data={waypointsList} keyExtractor={(item) => (waypoints[item]?.keyForList ?? waypoints[item]?.address ?? '') + item} shouldUsePortal - onDragEnd={updateWaypoints} + onDragEnd={(params) => { + updateWaypoints(params); + setSwipeEnabled?.(true); + }} + onDragStart={() => { + setSwipeEnabled?.(false); + }} ref={scrollViewRef} renderItem={renderItem} ListFooterComponent={ ```

[!important] Make sure to test your implementation with CONFIG.USE_REACT_STRICT_MODE set to false because this is how it will behave once on staging / production.

Regarding the alternative solution, the proposed change is not desirable in this case as we don't want to completely disable swiping between tabs while on the Distance request tab.

melvin-bot[bot] commented 1 week ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

ikevin127 commented 1 week ago

I think there are two ways to go about this issue:

  1. Do nothing and close the issue because it's a UI issue which does not affect / block the user from taking any action.
  2. Increase the bounty to $250 or even $500 depending on how urgent / important a fix is needed.

cc @twisterdotcom

twisterdotcom commented 1 week ago

Let's close. This is niche and obviously not super easy to fix. Thanks for the help @ikevin127.