[HOLD for payment 2023-09-13] [$500] Mobile distance confirmation screen initially starts on Atlantic City no matter what your start/finish destinations are

[JmillsExpensify]

Oddly, on iOS and Android the confirmation screen starts off panned on Atlantic City, no matter what start/finish destinations someone has entered.

Reproduction video below.

RPReplay_Final1693858456.MP4

I wasn't able to reproduce this on mWeb.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01347969a6fa0bd52c
• Upwork Job ID: 1698811053702803456
• Last Price Increase: 2023-09-04
• Automatic offers:
• jjcoffee | Contributor | 26496310
• Pujan92 | Contributor | 26496378
• situchan | Contributor | 26499786

[melvin-bot]

Current assignee @JmillsExpensify is eligible for the Bug assigner, not assigning anyone new.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01347969a6fa0bd52c

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

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

[melvin-bot]

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

[Pujan92]

Proposal

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

Confirmation route starts from incorrect point in distance flow

What is the root cause of that problem?

We are not providing the initialState in confirmation page for the MapView which we are providing for the distance page.

App/src/components/DistanceRequest.js

Lines 238 to 245 in 8150ce0

	<MapView
	accessToken={mapboxAccessToken.token}
	mapPadding={CONST.MAPBOX.PADDING}
	pitchEnabled={false}
	initialState={{
	zoom: CONST.MAPBOX.DEFAULT_ZOOM,
	location: CONST.MAPBOX.DEFAULT_COORDINATE,
	}}

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

We need to add initialState in the ConfirmedRoute page.

 initialState={{
      zoom: CONST.MAPBOX.DEFAULT_ZOOM,
      location: CONST.MAPBOX.DEFAULT_COORDINATE,
  }}

App/src/components/ConfirmedRoute.js

Line 92 in 8150ce0

<MapView

Simulator.Screen.Recording.-.iPhone.11.-.2023-09-05.at.03.21.49.mp4

OR

We can set the first waypointMarkers coordinate to the initialState.

initialState={{
      zoom: CONST.MAPBOX.DEFAULT_ZOOM,
      location: waypointMarkers[0].coordinate,
  }}

Simulator.Screen.Recording.-.iPhone.11.-.2023-09-05.at.03.21.14.mp4

[b4s36t4]

Proposal

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

Mobile distance confirmation screen initially starts on Atlantic City no matter what your start/finish destinations are

What is the root cause of that problem?

For web mapview we have a default start state which is San Fransico https://github.com/Expensify/react-native-x-maps/blob/c7d24166b7744e1e5310ddb49588ccd70c513ef2/src/MapView/MapView.web.tsx#L11

But for mobile we haven't defined initial state where to start the map.

https://github.com/Expensify/react-native-x-maps/blob/afabed2d2d85c14401354413cd9e5e0db2fdb4cb/src/MapView/MapView.tsx#L9

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

We can pass the initial state of location inside ConfirmedRoute just like as we do in DistanceRequest component.

App/src/components/DistanceRequest.js

Line 242 in d2bdaec

initialState={{

initialState={{
    zoom: CONST.MAPBOX.DEFAULT_ZOOM,
    location: CONST.MAPBOX.DEFAULT_COORDINATE,
}}

App/src/components/ConfirmedRoute.js

Line 92 in 1e0a249

<MapView

What alternative solutions did you explore? (Optional)

Or we can just define initial state in react-native-x-maps just as we do for web map

[MahmoudHemaid]

Proposal

Mobile distance confirmation screen initially starts on Atlantic City no matter what your start/finish destinations are

What is the root cause of that problem?

We calculate the map on screen focus which triggered after the screen is already visible so the it's not the initial load for the map

useFocusEffect(
    useCallback(() => {
        if (waypoints?.length && isIdle) {
            if (waypoints.length === 1) {
                cameraRef.current?.setCamera({
                    zoomLevel: 15,
                    animationDuration: 1500,
                    centerCoordinate: waypoints[0].coordinate,
                });
            } else {
                const {southWest, northEast} = utils.getBounds(waypoints.map((waypoint) => waypoint.coordinate));
                cameraRef.current?.fitBounds(northEast, southWest, mapPadding, 1000);
            }
        }
        return () => {
            setIsIdle(false);
        };
    }, [mapPadding, waypoints, isIdle]),
);

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

Add bounds to initial state and pass it from ConfirmedRoute

const waypointMarkers = getWaypointMarkers(waypoints);
const {southWest, northEast} = utils.getBounds(waypointMarkers      .map((waypoint) => waypoint.coordinate));
const initialMapState = {
    bounds: {
        paddingBottom: CONST.MAP_PADDING,
        paddingLeft: CONST.MAP_PADDING,
        paddingRight: CONST.MAP_PADDING,
        paddingTop: CONST.MAP_PADDING,
        ne: northEast,
        sw: southWest,
    }
};

Simulator.Screen.Recording.-.iPhone.13.-.2023-09-05.at.01.12.30.mp4

What alternative solutions did you explore? (Optional)

N/A

[melvin-bot]

📣 @MahmoudHemaid! 📣
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. 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.
2. 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.
3. 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>

[MahmoudHemaid]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~015ba45058db654948

[melvin-bot]

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

[melvin-bot]

📣 @jjcoffee 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer 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 📖

[hayata-suenaga]

@Pujan92 @b4s36t4

Do we have to refactor a lot if we want to avoid the flying behavior and just want the map on the confirmation page to display the correct location from the get-go?

[Pujan92]

No @hayata-suenaga ,
we just need to set below prop which starts from the first waypoint coordinate

initialState={{
      zoom: CONST.MAPBOX.DEFAULT_ZOOM,
      location: waypointMarkers[0].coordinate,
  }}

for a safer side we can make it
location: waypointMarkers[0].coordinate || CONST.MAPBOX.DEFAULT_COORDINATE

[hayata-suenaga]

No @hayata-suenaga ,
we just need to set below prop which starts from the first waypoint coordinate

initialState={{
zoom: CONST.MAPBOX.DEFAULT_ZOOM,
location: waypointMarkers[0].coordinate,
}}
for a safer side we can make it
location: waypointMarkers[0].coordinate || CONST.MAPBOX.DEFAULT_COORDINATE

Let's do this 🚀

[melvin-bot]

📣 @Pujan92 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer 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 📖

[melvin-bot]

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:

• [@situchan] The PR that introduced the bug has been identified. Link to the PR:
• [@situchan] 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:
• [@situchan] 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:
• [@situchan] Determine if we should create a regression test for this bug.
• [@situchan] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@JmillsExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

@JmillsExpensify, @Pujan92, @hayata-suenaga, @situchan Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

@JmillsExpensify, @Pujan92, @hayata-suenaga, @situchan 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[hayata-suenaga]

I think this one is waiting for payment

[hayata-suenaga]

waiting for payment

[melvin-bot]

@JmillsExpensify, @Pujan92, @hayata-suenaga, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@JmillsExpensify, @Pujan92, @hayata-suenaga, @situchan 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[hayata-suenaga]

still waiting for payment

[melvin-bot]

@JmillsExpensify, @Pujan92, @hayata-suenaga, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

[JmillsExpensify]

@situchan Would you mind completing the BZ checklist?

[JmillsExpensify]

Payment summary:

• Issue reporter: N/A
• Contributor: @Pujan92 $1,000 (incl. urgency bonus)
• Contributor+: @situchan $1,000 (incl. urgency bonus)

[JmillsExpensify]

Offer sent to @Pujan92

[Pujan92]

Accepted :)

[situchan]

• The PR that introduced the bug has been identified. Link to the PR: Modify MoneyRequestConfirmPage for Distance Request #25707
• 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: https://github.com/Expensify/App/pull/25707/files#r1349616494
• 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
• Determine if we should create a regression test for this bug.
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

I feel this doesn't require regression test. Wrong location was visible for a brief moment on first page load, but user ultimately sees correct route.

[situchan]

I already have accepted offer - #26726 (comment)

[Pujan92]

Accepted :)

Sry but I already accepted here.

[JmillsExpensify]

Thanks for clearing that up. Both offers paid out!