Initial Map Loading #668
Initial Map Loading #668
Conversation
![reviewify-code-review[bot]](https://avatars.githubusercontent.com/in/269822?s=60&v=4){kind=small}
| enabled: initialLoadTreeId.current != null, | ||
| onSuccess: ({ lng, lat, id }) => { | ||
| mapboxManager.setCenter({ | ||
| coords: { lng, lat }, |
There was a problem hiding this comment.
The TODO comment should be addressed or removed if it's no longer relevant.
| localStorage.setItem('lastVisitedUrl', window.location.href); | ||
| }); | ||
|
|
||
| setIsMapLoaded(true); |
There was a problem hiding this comment.
Consider wrapping setIsMapLoaded(true) in a separate useEffect with proper dependencies to avoid potential issues with the component lifecycle.
| zoom: 15, | ||
| }); | ||
| hasInitialFlyToFired.current = true; | ||
| // 1. If URL contains a valid id hash param, map ignores the pos in favor of coordinates of tree with id |
There was a problem hiding this comment.
Consider checking if 'lastVisitedUrl' is a valid URL before assigning it to 'window.location'.
mwpark2014
changed the base branch from
main
to
mwpark/feature/mapbox-camera-manager
mwpark2014
force-pushed
the
mwpark/feature/refactor-initial-load
branch
from
a883806 to
021126f
Compare
There was a problem hiding this comment.
Is this merge intentional?
Do you want to merge this into your other branch or do you want to stagger the PRs with mwpark/feature/mapbox-camera-manager going in first?
|
Lighthouse should be fixed now but Paul just made another change/revert. Need to rebase main rebase for lighthouse test to pass. |
Stagger the PRs with mwpark/feature/mapbox-camera-manager going in first |
| }, | ||
| { | ||
| retry: 0, | ||
| enabled: initialLoadTreeId.current != null, |
There was a problem hiding this comment.
Ah crap, just realized there's a bug in here in that this query will be repeated after the default cache/stale time runs out. If that happens, the map will be recentered at seemingly random times.
Maybe an alternative is having a hook for doing something just once after all conditions pass lol. Seems hacky but better than having a bunch of isInitiated flags laying around
Extract map dependency from components Add to the MapboxManager API Better method naming, deal with edge cases
mwpark2014
force-pushed
the
mwpark/feature/mapbox-camera-manager
branch
from
d41229b to
87d1ef2
Compare
mwpark2014
force-pushed
the
mwpark/feature/refactor-initial-load
branch
from
021126f to
053e147
Compare
mwpark2014
force-pushed
the
mwpark/feature/refactor-initial-load
branch
from
053e147 to
fe8662e
Compare
|
Is this still in draft mode? Are you still working on this? |
|
@mwpark2014 Is this ready for review or still in progress? Thanks |
Fixes #447
Builds off of https://docs.google.com/document/d/1MSSbkCo77VCE9tHZdWKAr8lfrBpRF_1AEMptInFjUBI/edit#
// 1. If URL contains a valid id hash param, map ignores the pos in favor of coordinates of tree with id
// 2. Else if URL contains valid pos hash param, map loads in with passed in LatLng and zoom
// 3. Otherwise, center map around the user’s last map url params if available
4. [Not implemented]. Otherwise, attempt to use IP address to infer user's location
5. If all else fails, default map to -99.08, 41.03 at global zoom
This PR: