Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PIMS-248] BUG: zoom wrong location upon marker click #1814

Merged
merged 8 commits into from
Oct 23, 2023

Conversation

dbarkowsky
Copy link
Collaborator

@dbarkowsky dbarkowsky commented Oct 13, 2023

🎯 Summary

PIMS-248:

Issue

Addressing a bug where clicking a marker causes the map to centre and zoom to the wrong location. Usually it would centre on the default centre of the province on the first click, then on the previous marker on subsequent clicks.

Cause

Issue was caused by a useEffect in Map.tsx that set the map's view before the information was updated.

Fix

Removed the useEffect. Added command for centre and zoom to the call when the data is fetched upon marker click in PointClusterer.tsx.

Testing

Click some markers on the map. You should no longer be zoomed to unexpected locations.

🔰 Checklist

  • I have read and agree with the following checklist and am following the guidelines in our Code of Conduct document.
  • I have performed a self-review of my code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation where required.
  • I have tested my changes to the best of my ability.
  • My changes generate no new warnings.

@dbarkowsky dbarkowsky self-assigned this Oct 13, 2023
@github-actions github-actions bot added the React label Oct 13, 2023
@dbarkowsky
Copy link
Collaborator Author

Fix is in place, but waiting to hear back from Lindsay on how she would like the centre and zoom behaviour to work before making my final changes and publishing for review.

@codeclimate
Copy link

codeclimate bot commented Oct 13, 2023

Code Climate has analyzed commit 117ca71 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

The test coverage on the diff in this pull request is 80.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 54.8%.

View more on Code Climate.

@dbarkowsky
Copy link
Collaborator Author

dbarkowsky commented Oct 18, 2023

Lindsay's suggestion was to remove the automatic centring and zooming.
Here is the code to restore this in case it is needed. It should be placed in PointClusterer.tsx, within the fetchProperty callback, inside the .then portion of getParcel and getBuilding.
Replace parcel with building where needed.
There is an option to only centre if desired, but it is commented out. If you only want to centre, uncomment that line, and remove the other two inside the if statement.

if (parcel.latitude !== '' && parcel.longitude !== '') {
    // mapInstance.setView({ lat: parcel.latitude, lng: parcel.longitude }); // just centre
    mapInstance.setView({ lat: parcel.latitude, lng: parcel.longitude }, MAX_ZOOM); // centre and zoom
    dispatch(setMapViewZoom(MAX_ZOOM)); // determines how markers are clustered; call this if you zoom
}

@github-actions github-actions bot added the Tests label Oct 18, 2023
@dbarkowsky dbarkowsky marked this pull request as ready for review October 18, 2023 17:08
@TaylorFries
Copy link
Collaborator

This update works well for me! Builds fine and no apparent issues.
We may want to test on a Windows machine to ensure that updates are consistent across OSs.

@dbarkowsky dbarkowsky merged commit 6b68ba9 into dev Oct 23, 2023
@dbarkowsky dbarkowsky deleted the PIMS-248-Zoom-Wrong-Location branch October 23, 2023 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants