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

feat: new empty state for additional map markers #1044

Merged
merged 4 commits into from
Jan 21, 2025

Conversation

emilyjablonski
Copy link
Collaborator

@emilyjablonski emilyjablonski commented Jan 13, 2025

This PR addresses #1049

  • Addresses the issue in full

Description

The issue has more details, but the PR creates two distinct empty states for the map - 1) there are no matching listings anywhere and the map is empty 2) there are no matching listings in the spot you're viewing in the map, but there are listings elsewhere, you just need to recenter

How Can This Be Tested/Reviewed?

Test each empty state - for the new state, see the new copy and the new button that will automatically re-fit the bounds of the map on the current markers (the only other time we fit bounds is on initial page load)

  1. No matching listings - Set a filter where no listings will match, and see the empty state indicate there are no matching listings and that you need to adjust your filters
  2. No visible listings - A. Scroll all the markers off the screen and see the new empty state indicating there are no markers in the search area, and click the button to reset your bounds. B. Zoom in on a single pin. Set a filter that will remove that pin from the view, but will keep other pins visible, like filtering on a different county. Click the button to reset your bounds.
Screenshot 2025-01-15 at 4 34 27 PM Screenshot 2025-01-15 at 4 34 35 PM

Author Checklist:

  • Added QA notes to the issue with applicable URLs
  • Reviewed in a desktop view
  • Reviewed in a mobile view
  • Reviewed considering accessibility / It resets the view, but could likely be improved with a specific screen reader announcement - it keeps the focus in the list which I think is preferable
  • Added tests covering the changes / Not having integration tests on this page would make this a hefty setup. We're refining tickets for more public side integration tests too! Frontend integration tests for the Doorway map/directory page #1050
  • Made corresponding changes to the documentation
  • Ran yarn generate:client and/or created a migration when required

Review Process:

  • Read and understand the issue
  • Ensure the author has added QA notes
  • Review the code itself from a style point of view
  • Pull the changes down locally and test that the acceptance criteria is met
  • Either (1) explicitly ask a clarifying question, (2) request changes, or (3) approve the PR, even if there are very small remaining changes, if you don't need to re-review after the updates

@emilyjablonski emilyjablonski marked this pull request as ready for review January 15, 2025 23:36
@jaredcwhite jaredcwhite added ready to merge Should be applied when a PR has been reviewed and approved and removed 1 review needed labels Jan 17, 2025
Copy link
Collaborator

@jaredcwhite jaredcwhite left a comment

Choose a reason for hiding this comment

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

:shipit:

@emilyjablonski emilyjablonski merged commit 6854a27 into main Jan 21, 2025
16 checks passed
@emilyjablonski emilyjablonski deleted the poc-zoom-out-auto-center branch January 21, 2025 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Should be applied when a PR has been reviewed and approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants