Background location tracking - GPS distance request#78279
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
|
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
melvin-bot
bot
requested review from
JmillsExpensify and
dukenv0307
and removed request for
a team
|
@dukenv0307 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] |
|
@JmillsExpensify please ignore, @dukenv0307 and @AndrewGable will review this |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c97eb17783
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <Button | ||
| onPress={onNext} | ||
| onPress={navigateToNextStep} | ||
| success |
There was a problem hiding this comment.
The Next button now directly calls navigateToNextStep without any check for distanceInMeters === 0, and showZeroDistanceModal is never set anywhere in this file. This means a user who stops a trip without moving (e.g., only one GPS point) can proceed without the zero‑distance warning that previously fired, leading to a 0‑distance request and missing feedback. Consider restoring a guard that shows the zero‑distance modal (or blocks navigation) when no distance was recorded.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks Mr. Computer
|
|
||
| const newGpsPoints = data.locations.map((location) => ({lat: location.coords.latitude, long: location.coords.longitude})); | ||
|
|
||
| addGpsPoints(newGpsPoints); |
There was a problem hiding this comment.
In addGpsPoints we're getting gpsDraftDetails, which can cause a race condition while setStartAddress is updating data
There was a problem hiding this comment.
Hmm I think it's not causing any problems here as in addGpsPoints we are not using startAddress, but it's true that this it could potentially result in a bug in the future, so I will pass gpsDraftDetails to addGpsPoints instead of reading it again, thanks
|
Tested this today and it looks mostly good. Two things:
You can see that the tooltip above shows up when it's 4.8 miles already and I've dismissed the tooltip already. Otherwise this looks great 👍 |
We only check if covered distance is equal to 0 to show this modal. You can see here I have different start and end addresses and 0.0 distance (recorded distance between start and end is 0.03mi) and it doesnt show me the modal when I tap 
Can we decide which conditions need to be met to stop showing the tooltip? Currently we show it all the time until user has successfully created GPS expense. I believe we want to change it so that when user closes it it won't be shown until the next GPS trip is started OR until user has created the GPS expense? Two examples:
Are these examples what we want to have? Or if user closes the tooltip then we don't want to show it ever again? |
Do we need to increase the amount of decimal places we show in the big distance display if we can in fact create expenses with distances of 0.01–0.09? It looks really weird to be able to create expense when the thing is showing 0.0 mi. |
I think showing two decimal places would make sense |
Curious what @AndrewGable thinks here. My loose take is that maybe we leave it as is but change the error message to be "Distance travelled must be greater than 0 miles". I guess we need to figure out what we want here. Should someone be able to submit distance for 0.01 miles? Part of me suggests the other way because that's still same as olddot, just with a better error message.
Hmm curious for @dannymcclain 's take here. My thinking is that we either do option 2 and never show it after you've created one GPS expense. Or we just show it at the start of each journey. Mostly what I wanted to not have is that you can see it when you come back to an in-progress trip. |
But still this message wouldn't be entirely correct, because 0.01 is still more than 0, right? Currently we just check if there is more than one GPS point recorded. In real-life scenario I don't think people would record 0.01 miles trips and blocking it and setting some boundary value may be problematic, because if we set it to 0.1 mi, then why not 0.5 mi? If 1 mi, then why wouldn't we allow 0.9 mi and so on. As we have location updates set to happen around every 0.06 mi (100 meters), I think it wouldn't be a bad idea to just start showing 2 decimal places instead of 1 and leave the rest as is, but it's just my take on this issue
I think that when the user closes the tooltip it's because they don't want it to ever be shown again in the same scenario, so maybe we only show it at the start of each trip (not again when user comes back to ongoing GPS trip from other screen) until user has closed the tooltip or has created a GPS expense? |
This sounds good to me. I think the user explicitly closing it or completing a GPS expense are both clear signals that we don't need to show it again. As far as the decimal places go, I'm really keen to hear what @AndrewGable (and @JmillsExpensify @trjExpensify) think about it. |
|
I don't have a super strong opinion. I think two decimals for distance feels a bit technical, but this rationale does make sense if that's the nature of how we record:
|
Okay, let's go with that then 👍 Show once, not again.
I agree with Tom on the technical. Feels so weird to me when driving in a car. How about we just leave it as is then unless someone else feels strongly. Mostly what I didn't like initially was that it said "Start and stop can't be the same" but in my instance the address was different. Maybe it's just about tweaking the language slightly, but also it's probably not a big deal. |
Yeah the problem probably was that it recorded just two points and the distance between them was <0.05mi (although I set location updates to happen every 0.06mi which should round off to 0.1 on the distance counter, they may vary slightly in the distance between them), so the start & stop addresses were different and it was showing 0.0mi @dubielzyk-expensify so for now I will post a follow-up PR soon with the change to the tooltip to not show it ever again if user closed it (even for new GPS trips). I like the discussion and we can continue it here if needed (regarding the number of decimals for the distance counter & tooltips), but I don't think it's blocking this PR and we can merge it to be able to finally put other PRs to review and get closer to the first release of this feature. cc: @AndrewGable |
|
Sounds good to me 👍 |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/AndrewGable in version: 9.3.5-0 🚀
|
|
@GCyganek GPS is not being counted even though it’s enabled and all permissions are granted. We even tried walking in real life ScreenRecording_01-21-2026.13-36-27_1.MP4Is this an issue, or do we need to do some additional settings? |
location updates should be triggered every 100 meters (=0.06mi) covered, which means even if you have a trip enabled for even 1 hour but not walked at least 100 meters from the starting point, there will be no distance update. Have you walked at least 100 meters? Could you also send a screenshot of location permissions set for the expensify app? Just to make sure it was started with precise location on and with |
|
@GCyganek tester tried to walk longer in an open place, it triggers the counting, but he can not tap on continue. 0.1 mil is too short? az_recorder_20260121_194628.mp4PXL_20260121_123822336.TS.mp4 |
|
@GCyganek It would be better to test this internally, as we’re having issues validating this PR |
Next button handling is not implemented yet. This PR only adds implementation of GPS location tracking, so as long as the location is tracked correctly everything is ok. |
|
Looking at the videos, it looks like location tracking is working, 0.1mi to appear takes longer than when you have km set as distance unit as 0.1mi is ~160meters so you have to walk longer to see change |
|
@GCyganek passed the PR, thanks for your help |
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.3.5-7 🚀
|
8 participants
Explanation of Change
Adds long-running background task for location tracking and processing location updates.
Translations for the Android notification title and body will be updated later today as I can't run the script myself
Fixed Issues
$ #77219 and #77220 and #77225
MOBILE-EXPENSIFY: https://github.com/Expensify/Mobile-Expensify/pull/13808
Tests
Using simulators:
iOS: Simulator has
Features > Locationoptions that when triggered can simulate location change eventsAndroid: Tap
...on the sidebar with additional simulator controls >Location> Create a route and tapPlay routeUsing physical devices:
You can either cover the distance for real or use applications that simulate location changes like Lockito
Start(grant all required location permissions if needed)StopWith backgrounding the app:
Start(grant all required location permissions if needed)StopOffline tests
QA Steps
Same as Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.ScrollViewcomponent to make it scrollable when more elements are added to the page.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-01-13.at.12.54.09.mov
iOS: Native
Screen.Recording.2026-01-13.at.11.58.23.mov