search

ParkingReformNetwork / parking-lot-map

Interactive map showing how much of downtowns are dominated by parking
https://parkingreform.org/parking-lot-map/
MIT License
1 stars 5 forks source link

Refactor updating scorecard #169

Closed tunglinn closed 8 months ago

tunglinn commented 8 months ago

Why this refactor? This simplifies when scorecard updates.

Before, there were many different cases in which a scorecard would update, like the dropdown changing, the user panning to a different city, or the user clicking on a city. Now, the scorecard only updates to the city closes to the center of the screen. So, when the dropdown changes, the map snaps to city, which will also automatically update the scorecard.

tunglinn commented 8 months ago

Nice! Although technically this isn't a refactor because there is a behavior change for end users. Refactors are when the end behavior is the same and you only change how the code is implemented.

There shouldn't bee a behavior change for the end user. Is there one that you noticed?

Eric-Arellano commented 8 months ago

Didn't it change like this?

Before, there were many different cases in which a scorecard would update, like the dropdown changing, the user panning to a different city, or the user clicking on a city. Now, the scorecard only updates to the city closes to the center of the screen. So, when the dropdown changes, the map snaps to city, which will also automatically update the scorecard.

My understanding of the code is that you now call snapToCity fewer times than before. But I didn't check this very rigorously.

tunglinn commented 8 months ago

Didn't it change like this?

Before, there were many different cases in which a scorecard would update, like the dropdown changing, the user panning to a different city, or the user clicking on a city. Now, the scorecard only updates to the city closes to the center of the screen. So, when the dropdown changes, the map snaps to city, which will also automatically update the scorecard.

My understanding of the code is that you now call snapToCity fewer times than before. But I didn't check this very rigorously.

tunglinn commented 8 months ago

Yes, but the end user experience doesn't/shouldn't change.