Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@QichenZhu Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
I'm OOO next week so if anything happens here @jmusial will take over |
dylanexpensify
left a comment
There was a problem hiding this comment.
Looks good from a product perspective 👍
|
Thanks! I'll test this in the morning. |
|
@QichenZhu updated PR Screen.Recording.2026-09-23.at.11.21.32.mov |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid-native-1.movandroid-native-2.movandroid-native-3.movandroid-native-4.movAndroid: mWeb ChromeiOS: HybridAppios-native-1.movios-native-2.movios-native-3.movios-native-4.moviOS: mWeb SafariMacOS: Chrome / Safari |
| setRemountKey((key) => key + 1); | ||
| }; | ||
|
|
||
| usePolling(checkLocationServices, CONST.TIMING.LOCATION_UPDATE_INTERVAL, true, CONST.TIMING.USE_DEBOUNCED_STATE_DELAY); |
There was a problem hiding this comment.
My understanding is that since GPS is already running continuously in the background, the extra battery usage from the polling timer should be small. Is it right?
| // Determines if map can be panned to user's detected location without bothering the user. It will return | ||
| // false if user has already started dragging the map or if there are one or more waypoints present | ||
| // and the GPS trip is not active or the foreground location permissions are not granted. | ||
| const shouldFollowUserLocation = !userInteractedWithMap && (noWaypoints || isTrackingGPS) && foregroundLocationPermissionsGranted; |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| setLastLocation({longitude: coords.longitude, latitude: coords.latitude}); | ||
| }; | ||
|
|
||
| const shouldFollowFallbackLocation = noWaypoints && !foregroundLocationPermissionsGranted; |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| /> | ||
|
|
||
| {/** Show fallback location if foreground location permissions are not granted */} | ||
| {!foregroundLocationPermissionsGranted && ( |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| // Only spend the immediate transition once follow mode has actually been reached. A transition | ||
| // that ended in failure (e.g. no location fix arrived) also reports `from.kind === 'transition'`, | ||
| // and clearing the flag there would leave the camera with the default, time-boxed transition and | ||
| // nothing to fall back on, because the JS layer discards every camera stop while following. |
There was a problem hiding this comment.
I don't quite understand this comment. Could you rephrase it?
|
NAB: The map doesn't draw a track after the location changes. Same issue in staging. nab.mov |
|
|
||
| type GPSMapViewContentProps = Omit<GPSMapViewProps, 'accessToken'> & { | ||
| /** Whether the foreground location permissions are granted */ | ||
| foregroundLocationPermissionsGranted: boolean; |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
NVM, the parent component guarantees the value is not null, so the child doesn't need to handle the case.
|
@QichenZhu sorry didn't have time today to address comments - will do tomorrow |
|
@jmusial do you want me to hold off until you address the last PR comments? Looks like they're mostly small stuff like rephrasing comments |
Explanation of Change
The GPS map unmounts when the app goes offline, but its camera state lived in
GPSMapViewand survived that unmount, so a rebuilt map inherited stale state. Moved the map and that state into a newGPSMapViewContentso a remount reseeds it.Fixed Issues
$ #99643
PROPOSAL: N/A
Tests
iOS and Android native app:
Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2026-09-17.at.15.33.50.mov
iOS: Native
Screen.Recording.2026-09-17.at.15.27.20.mov