Fix map panning issues related resource info modal #539
Conversation
This fixes the issue where opening a site, then clicking "Near Me", then closing the info modal was panning back to the prior site, whereas it should have stayed at the "Near Me" site. This is "issue C" as discussed in the comments of GitHub issue #527.
The name change reflects the fact that currently as a user may have panned around the map by clicking and dragging the map, the `mapCenter` state was not updated. Instead, the state is updated when e.g. a pin is clicked, or the "Near Me" button is clicked.
src/reducers/filterMarkers.js
Outdated
| @@ -129,7 +130,7 @@ export default (state = initialState, act) => { | |||
| : { | |||
| ...state, | |||
| selectedPlace: state.allResources[act.selectedPlace], | |||
| showingInfoWindow: true | |||
| showingInfoWindow: true // debuggin! why is this here, and not in the other branch? | |||
There was a problem hiding this comment.
Just checking, does this comment still need to be addressed or can it be removed?
There was a problem hiding this comment.
Ack (no pun intended!) Thanks for catching this, it was a self-comment while I was working, which has now been removed.
To clarify, by "branch" I meant the if branch in filterReducers, which did not set the selected place, while the else branch did. After a bit more digging, I could not find any instance where we are actually using the resource ID to set the place, hence the if-else seemed unnecessary, removing it altogether removed the need to worry about setting the selected place in both branches.
TL;DR - I have removed the if-else and that comment in the reducer, and also transitioned to the createAction syntax as part of the recent guidelines. Also, some of the commit changes are being caused by prettier formatting, sorry for any confusion!
Removed unnecessary if-else condition from the reducer, and transitioned to the `createAction` syntax.
|
I believe your last comment was addressed. Please let me know if I've missed anything! |
Nope, your change makes sense! I'm thinking the check for typeof in the old |
Change Summary
Addresses #527. Introduces
lastResourcePanin redux state to supersede priormapCenterin redux, andcurrentLat/currentLoncomponent state in<ReactGoogleMaps>, which helps properly track the last pan caused by the user clicking a pin or on the "Near Me" button.Change Reason
Fixes various issues as described in detail in issue #527, where the map was panning when it shouldn't be, and wasn't panning when it should be, depending on the scenario.
Related Issue: #527