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

Clicking on city boundary will pull up scorecard #139

Merged
merged 6 commits into from
Nov 19, 2023

Conversation

tunglinn
Copy link
Collaborator

@tunglinn tunglinn commented Nov 3, 2023

  • each <path> for city boundary now has an id attribute

Current map zoom required to allow click is 7, which looks like this.
image

And, this zoom and beyond will not allow you to click on cities.
image

@tunglinn
Copy link
Collaborator Author

tunglinn commented Nov 4, 2023

Birmingham was added to city-boundaries.geojson as a LineString instead of a Polygon (so the empty space inside the boundary was not being detected on clicks).

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great!

@@ -571,67 +571,69 @@
"id": "birmingham-al"
},
"geometry": {
"type": "LineString",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find. Looks like this is the only instance of it.

Comment on lines 211 to 212
cityToggleElement.value = cityId;
setMapToCity(map, cityId, cities[cityId]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ik this is still a draft so pardon if this is premature. We'll still want to only do this if the zoom is a certain value. Otherwise clicking on the city from zoomed out will be noisy.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you click a city, it zooms in onto the city (similar to if you select it from the dropdown). I think this resolves the noise you're talking about?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite. Imagine you're super zoomed out on the map, so the cities are tiny dots. You click on it because let's say you're on a phone and the touch targets aren't big—but really you were just trying to zoom in or move around the map!

We should only change the city if the zoom level is below a certain threshold.

@tunglinn tunglinn marked this pull request as ready for review November 16, 2023 01:49
@Eric-Arellano
Copy link
Contributor

👏

@tunglinn tunglinn merged commit dcbddda into ParkingReformNetwork:main Nov 19, 2023
1 check passed
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