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

Alert if user clicks Plan Trip with invalid mode selection #990

Merged
merged 4 commits into from
Sep 14, 2023

Conversation

binh-dam-ibigroup
Copy link
Collaborator

Description

This PR adds an alert shown when user clicks "Plan Trip" with invalid modes selected (e.g. "car" alone).
No new alerts are shown when planning is triggered outside of clicking the "Plan Trip" button.

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

@amy-corson-ibigroup amy-corson-ibigroup left a comment

Choose a reason for hiding this comment

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

Very helpful! Thanks Binh! A couple suggestions but nothing blocking

@@ -0,0 +1,5 @@
export const RoutingQueryResult = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be appropriate to rename to ROUTING_QUERY_RESULT? Even just to make it a bit more distinct from routingQueryResult in batch-settings.tsx

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Instead of changing the case, I picked a (better) name in 8a0e6c9.

i18n/en-US.yml Outdated
@@ -227,6 +227,7 @@ components:
modeSelectorLabel: Select a transit mode
BatchSettings:
destination: destination
invalidModeSelection: Cannot plan a trip using the selected modes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add another sentence like "Try including transit in your mode selection" just to point people in the right direction?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea! Added in c72cbc7.

Copy link
Member

@AdrianaCeric AdrianaCeric left a comment

Choose a reason for hiding this comment

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

This works great! I'm wondering if this alert could also be shown when the plan trip button isn't selected. For instance, when planning trips I almost never click the green button. I usually just select the start and end positions on the map

@binh-dam-ibigroup binh-dam-ibigroup merged commit d7f0c2b into dev Sep 14, 2023
5 checks passed
@binh-dam-ibigroup binh-dam-ibigroup deleted the alert-on-forbidden-combo branch September 14, 2023 19:19
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