[Waiting for C+ payment] [$500] Android - Wrong location briefly shows before San Francisco when first open Distance request page

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

---

Action Performed:

1. Click on FAB > Request money
2. Click Distance tab
3. Observe map view

Expected Result:

Shows San Francisco as default location

Actual Result:

Weird location briefly shows before directing to San Francisco

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.63.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
Notes/Photos/Videos: Any additional supporting documentation

bug.2.mov

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0106c2a9461a735378
• Upwork Job ID: 1710683488590667776
• Last Price Increase: 2023-10-07

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014793e3b4669749b9

[melvin-bot]

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

[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 @kadiealexander (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[akinwale]

Proposal

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

Wrong location briefly shown on the map before the default initial location, San Francisco the first time the Distance request page is open.

What is the root cause of that problem?

This seems to be an issue specific to how the default location is set with the Mapbox SDK on Android. It is easily reproducible after launching the app for the first time. It seems when the camera (used for displaying the map viewport) is initialised for the first time, it has to move the camera center to the new location from Atlantic City (the default center if none is set). On subsequent attempts to open the distance request screen, San Francisco remains the center, so the camera does not have to move.

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

There is a callback to set the map idle state, which indicates that the map is ready for interaction and updates. We can render an overlay (or the blocking view, but rendered on top of the map) in DistanceRequest before the first call to set the map idle state, and then hide the overlay when the setMapIdle handler is called.

This overlay will hide the initial camera movement from Atlantic City to San Francisco, and then when the map is ready (idle), the overlay gets hidden. A loading indicator or animation can be added to the overlay, if required.

This can be achieved with the following steps.

1. In MapView.tsx, add an onMapIdle or onMapReady prop which will be a method. Add the prop type to MapViewTypes.ts: onMapReady: () => void;

2. Update the setMapIdle method.

const setMapIdle = (e: MapState) => {
    if (e.gestures.isGestureActive) return;
    setIsIdle(true);
+    if (onMapReady) {
+        onMapReady();
+    }
};

3. In DistanceRequest, add a state variable, isMapReady.

const [isMapReady, setIsMapReady] = useState(false);

4. Update the rendering block for the MapView component in DistanceRequest with the overlay, and implement the handler for the onMapReady prop.

{!isOffline && Boolean(mapboxAccessToken.token) ? (
+  <>
      <MapView
          ...
+          onMapReady={useCallback(() => {
+              if (!isMapReady) {
+                  setIsMapReady(true);
+              }
+          }, [])}
      />
+      {!isMapReady && <View style={styles.mapViewOverlay} />}
+  </>
)

5. Add overlay styles. These can be tweaked.

mapViewOverlay: {
    flex: 1,
    position: 'absolute',
    left: 0,
    top: 0,
    borderRadius: 16,
    width: '100%',
    height: '100%',
    backgroundColor: themeColors.cardBG,
    ...spacing.m4,
},

Since similar changes will also need to be made to the ConfirmedRoute component, we can consider creating a separate component for this, MapViewWithOverlay which incorporates the necessary logic.

This only happens on Android, so we should consider creating a platform-specific index.android.js implementation with the proposed DistanceRequest changes. The setMapIdle handler does not get triggered on web or desktop.

The included demo video below shows what it would look like with the BlockingView as a child of the overlay.

What alternative solutions did you explore? (Optional)

I tried setting the animationMode for the camera to none, with the assumption that the camera's initial movement from Atlantic City to San Francisco was animated, but this did not have any effect.

demo-27087-blocking-view.webm

[ZhenjaHorbach]

Proposal

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

During launching the map, we can see the default position of the camera with default coordinates

What is the root cause of that problem?

The main problem is the coordinate value, which can be undefined for a short time during initialization of the map.
As a result, the default position from the library will be used.

Default position camera :

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

In Mapbox.Camera, we don't need to pass dynamic values inside the defaultSettings.centerCoordinate.

App/src/components/MapView/MapView.tsx

Lines 70 to 76 in b11bddc

	<Mapbox.Camera
	ref={cameraRef}
	defaultSettings={{
	centerCoordinate: initialState?.location,
	zoomLevel: initialState?.zoom,
	}}
	/>

Instead, we can use centerCoordinate and use constant with value [-122.4021, 37.7911](San Francisco) for defaultSettings.centerCoordinate.

                <Mapbox.Camera
                    ref={cameraRef}
		    centerCoordinate={initialState?.location}
                    defaultSettings={{
                        centerCoordinate: [-122.4021, 37.7911],
                        zoomLevel: initialState?.zoom,
                    }}
                />

What alternative solutions did you explore? (Optional)

NA

[sonialiap]

@mollfpr we have a couple proposals, what do you think?

[mollfpr]

@akinwale @ZhenjaHorbach Why it's only happening on Android?

The main problem is the coordinate value, which can be undefined for a short time during initialization of the map.

@ZhenjaHorbach Is this a problem in the upstream or our component?

I have a hard time confirming the issue. Does anyone have a suggestion?

[ZhenjaHorbach]

@mollfpr
I think if we really want to, this problem can be reproduced on any platform ))
I was only able to reproduce this on a Mac in power saving mode on emulator

But perhaps the map on Android is much more demanding on hardware compared to other platforms
As a result, it takes more time to redraw the map

[akinwale]

@akinwale @ZhenjaHorbach Why it's only happening on Android?

The main problem is the coordinate value, which can be undefined for a short time during initialization of the map.

This is not actually a fix for the issue. I tested this and it is still reproducible even after setting the coordinates directly in MapView.tsx.

I have a hard time confirming the issue. Does anyone have a suggestion?

It seems that the Mapbox SDK for Android is not as performant as other platforms probably due to different device configurations in terms of processor speed and available memory.

I can easily reproduce the issue in the Android 11 emulator. In order to reproduce the behaviour reliably, close the app completely (also remove from recents), relaunch the app and then try to create a Distance request. You will notice the map moving from the initial location the first time the map loads. This only happens the first time. If you try to create another Distance request during the same app session, it will be centered on San Francisco already.

[ZhenjaHorbach]

@akinwale
can you record a video, please ?
Because if we deleted the default coordinate values
Then we will see the same location as in the bug description

Therefore, it is logical to overwrite this value with the one we need

[akinwale]

@akinwale can you record a video, please ? Because if we deleted the default coordinate values Then we will see the same location as in the bug description

Therefore, it is logical to overwrite this value with the one we need

default-location-flash.webm

You'll notice around the 0:31 / 0:32 mark that the default location (Atlantic City) flashes and renders for a bit before the map moves to the center coordinate (San Francisco).

[ZhenjaHorbach]

Oh yeah
Sorry )
Apparently my version is not perfect enough
But I still think that we need to find the place where the coordinates are initialized
Perhaps this is done in the library itself

[mollfpr]

I'm okay with @akinwale proposal to cover the map component until it's ready (idle). I tried to find a similar issue on the upstream, but no one cares much about it, and it's tough to notice.

Also, this issue polishes the map render and does not block the user from using the distance request feature.

I would like to hear the opinion of the internal engineer. So I'm recommending to consider @akinwale proposal.

🎀 👀 🎀 C+ reviewed!

[melvin-bot]

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

[mollfpr]

@sonialiap Add a note for the speed bonus calculation.

The assignment is on 14th Sept GMT+7, and the PR merged on 15th Sept GMT+7.

---

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
[@mollfpr] 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:

No offending PRs. This is an improvement for the Request money distance feature.

[@mollfpr] 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:

The regression step should be enough.

[@mollfpr] Determine if we should create a regression test for this bug.
[@mollfpr] 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.

1. On Android native, close the app and remove the app from the recent menu
2. Launch the app
3. Tap the global create menu, and select Request Money
4. Select the Distance tab if it is not already selected
5. Verify that the map displays San Francisco and there is no weird jumping after the blocking view overlay is automatically hidden
6. Select Start and Finish waypoints
7. Tap on Next and then select the target workspace
8. Verify that there is no weird jumping on the map on the confirmation screen after the blocking view overlay is automatically hidden. The map may zoom in on the route after the overlay is hidden, which is the expected behavior
9. 👍 or 👎

[kadiealexander]

Payouts due:

• Issue Reporter: $50 @situchan
• Contributor: $500 + $250 bonus @akinwale
• Contributor+: $500 + $250 bonus @mollfpr

Eligible for 50% #urgency bonus? Yes (PR was merged within 24 hours)

Upwork job is here.

[kadiealexander]

Upwork contracts sent to @situchan and @akinwale!

@mollfpr please let us know if you're paid via Upwork or Newdot

[akinwale]

Upwork contracts sent to @situchan and @akinwale!

@mollfpr please let me know if you're paid via Upwork or Newdot

Accepted. Thanks!

[mallenexpensify]

@JmillsExpensify , can you confirm that @mollfpr is setup for payment via NewDot? If so I'll drop a note in #bug-zero with an updated list of everyone

[melvin-bot]

@tylerkaraszewski, @akinwale, @sonialiap, @mollfpr, @kadiealexander Huh... This is 4 days overdue. Who can take care of this?

[kadiealexander]

Not overdue. @mollfpr please let us know how you'd prefer to be paid.

[mollfpr]

@kadiealexander I would love to get this pay in NewDot, but I am still waiting for the confirmation #27087 (comment).

[mallenexpensify]

Confirming @mollfpr can be paid via NewDot. The SO is updated and the associated sheet.

[mollfpr]

@mallenexpensify I guess I can create the request in the NewDot now?

[mallenexpensify]

Yes @mollfpr , please do, based on the instructions I sent you last week. Thx

[JmillsExpensify]

@JmillsExpensify , can you confirm that @mollfpr is setup for payment via NewDot? If so I'll drop a note in #bug-zero with an updated list of everyone

Yes.

[JmillsExpensify]

$750 payment approved for @mollfpr based on BZ summary.

[melvin-bot]

@tylerkaraszewski @akinwale @sonialiap @mollfpr @kadiealexander this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is today
• If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0106c2a9461a735378

[melvin-bot]

Current assignees @akinwale and @mollfpr are eligible for the Internal assigner, not assigning anyone new.