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

Remove deprecated setZoneAssignments #220

Closed
wants to merge 1 commit into from
Closed

Conversation

mapmeld
Copy link
Member

@mapmeld mapmeld commented Dec 9, 2024

Mentioned this on Slack - now that we moved to an event listener, setZoneAssignments is only called by the zone picker. We don't think this is necessary anymore now that the mouse exiting the map should apply the changes.

The calls were added to the picker in this commit and PR:
8dc9328
#129 (likely copying the Picker)

@nofurtherinformation
Copy link
Collaborator

Hey @mapmeld ! Thanks for this!

I reviewed and realized that the addition of zone updates within mapStore.selectMapfeatures was an error I introduced. It works and keeps the state highly synced with the map, but causes some performance and excess request issues.

I think the right way to handle zone updates (for now at least) is to only mutate the accumulatedGeoids set and handle any map rendering and tile logic (eg. clientside population) and then at the end of a paint event update the zone assignments.. This direction made more sense in the population chart enhancements and I think will simplify undo/redo in terms of number of events firing off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants