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

fix: make drawer accessible in mobile landscape #1011

Merged
merged 7 commits into from
Dec 19, 2024
Merged

Conversation

emilyjablonski
Copy link
Collaborator

@emilyjablonski emilyjablonski commented Dec 16, 2024

#1012

Description

Out of bash, we need on mobile devices in landscape mode looking at the map to be able to access the listings list. On production right now, you can see a little sliver, and on staging, you can't see it at all. This PR makes it so the list is accessible, but doesn't improve upon the experience.

This PR:

  • Fixes the slight scrolling behavior in the old and the new map
  • Creates more space on landscape by combining the filter bar and results bar in the old map
  • Slightly reduces spacing around the logo on mobile

How Can This Be Tested/Reviewed?

Using your mobile device, visit the listings map on landscape, and ensure the list is accessible. You can compare the experience to production. There are instructions here for visiting your local stack in a mobile device. You can also toggle landscape mode on and off in the browser's mobile emulator.

Author Checklist:

  • Added QA notes to the issue with applicable URLs
  • Reviewed in a desktop view
  • Reviewed in a mobile view
  • Reviewed considering accessibility
  • Added tests covering the changes
  • 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 December 16, 2024 23:48
@ColinBuyck
Copy link
Collaborator

Documenting offline discussion. Given this is my current experience on mobile landscape with the fix, this is on hold to update the filter and total results to show on the same line and hopefully provide more room to use the drawer 🐧
IMG_7DBA8F899460-1

Copy link
Collaborator

@ColinBuyck ColinBuyck left a comment

Choose a reason for hiding this comment

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

Functionality looks good 🦤 assuming that submit app test is flakiness

@ColinBuyck ColinBuyck added ready to merge Should be applied when a PR has been reviewed and approved and removed 1 review needed labels Dec 18, 2024
Copy link

cypress bot commented Dec 18, 2024

Doorway Cypress Public    Run #47

Run Properties:  status check passed Passed #47  •  git commit 21279598b1 ℹ️: Merge 17e3e5f68d34a0850cbb852837ed9db97c4b5015 into d1c4c101cf52c3c5ef9732515b44...
Project Doorway Cypress Public
Branch Review landscape-map
Run status status check passed Passed #47
Run duration 01m 22s
Commit git commit 21279598b1 ℹ️: Merge 17e3e5f68d34a0850cbb852837ed9db97c4b5015 into d1c4c101cf52c3c5ef9732515b44...
Committer Emily Jablonski
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 1
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
View all changes introduced in this branch ↗︎

@emilyjablonski emilyjablonski merged commit 4b4727c into main Dec 19, 2024
16 checks passed
@emilyjablonski emilyjablonski deleted the landscape-map branch December 19, 2024 16:45
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