refactor: port iOS location architecture to Android #568
Conversation
| var hasPermission by mutableStateOf(false) | ||
|
|
||
| /** | ||
| * Attach the event handlers for this [LocationDataManager] in the context of the current |
There was a problem hiding this comment.
Thanks for the comment, much appreciated!
| import kotlinx.coroutines.flow.MutableStateFlow | ||
| import kotlinx.coroutines.flow.asStateFlow | ||
|
|
||
| open class LocationDataManager { |
There was a problem hiding this comment.
Do we actually want this class to be open? My reading of the comment is that we only want to call the rememberLocationManager function, which seems incompatible with extending this. In general it would be nice if we could throw an error if LocationDataManager is initialized twice, rather than relying on the next dev to have read your comment.
There was a problem hiding this comment.
This has to be open so that the MockLocationDataManager can inherit from it. I could've made LocationDataManager an interface instead, but I don't like the pattern of making everything a redundant interface; that said, we already do that for all our dependency injection repositories, so if you'd rather I also do that here for consistency I could make that change.
There was a problem hiding this comment.
Nah, I didn't realize it was for testing. That's fine IMO
| @@ -282,17 +292,108 @@ fun HomeMapView( | |||
|
|
|||
| val context = LocalContext.current | |||
|
|
|||
| val locationProvider = remember { | |||
| object : LocationProvider { | |||
There was a problem hiding this comment.
Could we move this to it's own file? HomeMapView is pretty big already, I think it would help readability to pull this out. Nonblocking!
|
|
||
| DisposableMapEffect { map -> | ||
| val listener = | ||
| object : OnMoveListener, OnScaleListener, OnShoveListener { |
There was a problem hiding this comment.
I also think this would benefit from being moved out of the file
Summary
Ticket: 🤖 | Map | Map => Nearby Transit location sharing architecture
The log gets spammed with "sending message to a Handler on a dead thread" via some Google Play location stuff, but only if location permission was already granted when the app first launched. I tried to investigate this, but I don't think anything is actually running on a different thread than it's supposed to, and it doesn't appear to be interfering with the actual functionality.
android
Testing
Manually verified that both follow puck and manually select states properly track the current location.