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

Prevent mobile view change on location clear #1085

Merged
merged 14 commits into from
Dec 15, 2023

Conversation

amy-corson-ibigroup
Copy link
Contributor

@amy-corson-ibigroup amy-corson-ibigroup commented Nov 30, 2023

Description:
Currently when you clear a location field in mobile view, it resets the view back to the form. This PR changes that so now using the clear location button just clears the location.

This PR also updates the mobile bar back button to be web accessible, and turns it on for the location field.

PR Checklist:

  • Does the code follow accessibility standards (WCAG 2.1 AA Compliant)?
  • Are all languages supported (Internationalization/Localization)?
  • Are appropriate Typescript types implemented?

Copy link
Contributor

@daniel-heppner-ibigroup daniel-heppner-ibigroup left a comment

Choose a reason for hiding this comment

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

I think we do need a close or back button here. The browser back button doesn't work to go back to the trip form. :( Sorry

Copy link
Contributor

@daniel-heppner-ibigroup daniel-heppner-ibigroup left a comment

Choose a reason for hiding this comment

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

Ooo yes nicely done! Thanks!

@binh-dam-ibigroup
Copy link
Collaborator

@amy-corson-ibigroup Do we need to fix some of the Percy tests?

@amy-corson-ibigroup
Copy link
Contributor Author

@binh-dam-ibigroup Fixed!

@amy-corson-ibigroup
Copy link
Contributor Author

As @binh-dam-ibigroup pointed out, the mobile nav bar already has a back arrow, so I've used that instead of creating my own! I've also updated the back arrow to be web accessible but changing it from a div to a button and adding an aria-label

Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

As @binh-dam-ibigroup pointed out, the mobile nav bar already has a back arrow, so I've used that instead of creating my own! I've also updated the back arrow to be web accessible but changing it from a div to a button and adding an aria-label

Nice fix!

lib/components/mobile/mobile.css Show resolved Hide resolved
lib/components/mobile/navigation-bar.js Show resolved Hide resolved
@amy-corson-ibigroup amy-corson-ibigroup merged commit cc5b6a9 into dev Dec 15, 2023
9 checks passed
@amy-corson-ibigroup amy-corson-ibigroup deleted the clear-location-mobile-view branch December 15, 2023 20:07
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.

3 participants