feat(HomeMapView): center on user's current location #38
Conversation
| var sut = HomeMapView(locationDataManager: locationDataManager) | ||
|
|
||
| let hasAppeared = sut.on(\.didAppear) { _ in | ||
| XCTAssertNotNil(sut.viewport.followPuck) |
There was a problem hiding this comment.
this test is skipped - I'm guessing that there is something going on in mapbox to not set the viewport to followPuck before there is a known location, so this doesn't happen until the location update is published.
iosApp/iosApp/ContentView.swift
Outdated
| @@ -51,6 +47,15 @@ struct ContentView: View { | |||
| } | |||
| } | |||
|
|
|||
| struct _HomeMapView: View { | |||
There was a problem hiding this comment.
I didn't want to init the locationManager inline in ContentView, which would have re-init every time the search text changed.
| "revision" : "67319287749b83f39dcc2f20edd520c610c012fd", | ||
| "version" : "0.9.10" | ||
| "revision" : "a1422d4749ccf7f32b1d14a9ec19ec9a0b0fd337", | ||
| "version" : "0.9.9" |
There was a problem hiding this comment.
Was this downgrade intentional?
There was a problem hiding this comment.
Nope, thanks for catching
There was a problem hiding this comment.
This imageset (and Image alongside it) appears not to contain an image at all; is this necessary?
There was a problem hiding this comment.
Ah yep none of these image sets are necessary - I was playing around with them in earlier iterations.
There was a problem hiding this comment.
This image appears to be missing the imageset metadata that location-puck has.
iosApp/iosApp/ContentView.swift
Outdated
| @@ -51,6 +47,15 @@ struct ContentView: View { | |||
| } | |||
| } | |||
|
|
|||
| struct _HomeMapView: View { | |||
| @StateObject var locationManager: LocationDataManager = .init(distanceFilter: 1) | |||
| var body: some View { | |||
There was a problem hiding this comment.
This indentation looks incorrect to me. Do you have pre-commit properly configured?
There was a problem hiding this comment.
Ah, I did a --no-verify commit at some point along the way. Pushed a cleanup
KaylaBrady
force-pushed
the
kb-map-location
branch
from
d3d0ffe to
96325cf
Compare
…ht/dark mode to prevent timeouts This test was taking 53 seconds on my machine because it really runs 8 tests. I don't think we need to assess launch time for each orientation
this test passes locally, but runs in CI. punting to figure out why
| @@ -40,6 +43,9 @@ | |||
| }, | |||
| { | |||
| "parallelizable" : true, | |||
| "skippedTests" : [ | |||
| "IosAppUITestsLaunchTests\/testLaunch()" | |||
There was a problem hiding this comment.
Skipping this test after discussion at standup - ticket to revisit here.
Summary
Ticket: Initial map setup
What is this PR for?
Center's the map on the user's current location when we have it.
In earlier commits I tried doing some custom logic based on this issue which describes high CPU usage when using the default .followPuck viewport.
I replicated that high usage (~40% while idle). However, when implementing custom follow puck logic instead the location-following behavior was pretty jumpy, even when using transitions. I opted to stick with the default
.followPuckimplementation for now with the idea that we can revisit if performance ends up being a problem.Testing
What testing have you done?
I tested this out on a local device and moved around, confirming that the map follows my location.
I wrote a test to try to assert this is the case but ended up skipping it - I think there is something finicky about the timing of location updates that is causing this to fail. I think going back to commit b68bfe5 would be easier to test.