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.

Sign up for GitHub

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

Jump to bottom

Scorecard updates to city in view or closest to center of map #143

Merged
merged 8 commits into from
Dec 10, 2023
Merged

Scorecard updates to city in view or closest to center of map #143

merged 8 commits into from
Dec 10, 2023

Conversation

tunglinn
Copy link
Collaborator

@tunglinn tunglinn commented Nov 21, 2023 •
edited
Loading

Resolves #112

@tunglinn
Copy link
Collaborator Author

Right now, only the scorecard updates. Should the dropdown also change?

@tunglinn tunglinn mentioned this pull request Nov 21, 2023
Merged
2 tasks
@Eric-Arellano
Copy link
Contributor

Should the dropdown also change?

I think probably? Otherwise you get into a weird UI state where the UI components aren't in sync. What do you think?

@tunglinn
Copy link
Collaborator Author

Should the dropdown also change?

I think probably? Otherwise you get into a weird UI state where the UI components aren't in sync. What do you think?

I agree. My concern is how do we differentiate between changes in the dropdown by the user versus by a function. Because if we don't, then changing the dropdown will also 'snap' the map view to the other city.

@Eric-Arellano
Copy link
Contributor

then changing the dropdown will also 'snap' the map view to the other city.

we want that, right? That's the current behavior.

@tunglinn
Copy link
Collaborator Author

we want that, right? That's the current behavior.

Yes, for switching cities via the dropdown.
But if the dropdown updates when the user is dragging to another city, then the same setMapToCity() is called and snaps view to city, which will make it difficult for users to just drag around the map.

@tunglinn tunglinn marked this pull request as ready for review December 1, 2023 02:13
@tunglinn tunglinn changed the base branch from lazy-loading to main December 1, 2023 17:42
Eric-Arellano
Eric-Arellano approved these changes Dec 4, 2023
View reviewed changes
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.

Yo this is so awesome! Great work! I love the UX of this when you scroll around the map. It's so intuitive and fluid.

tests/app/setUpSite.test.js Outdated
await page.mouse.up();
// Click on Birmingham boundary.
await dragMap(page, 200);
// Click on Birmingham ;boundary.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Click on Birmingham ;boundary.
// Click on Birmingham boundary.

@tunglinn tunglinn merged commit 2018c0c into ParkingReformNetwork:main Dec 10, 2023
1 check passed
tunglinn added a commit that referenced this pull request Dec 19, 2023
@tunglinn
Lazily load parking lots (#141)
51642eb 
Right now, parking lots will only load if you use the dropdown. However,
a better implementation is to "Load when dragged into view". Might wait
for #143 to implement.
To-do:
- [x] Load when dragged into view
- [x] Tests

---------

Co-authored-by: tunglinn <tunglin@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Eric-Arellano Eric-Arellano Eric-Arellano approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dragging map to city should pull up its scorecard
2 participants
@tunglinn @Eric-Arellano