-
Notifications
You must be signed in to change notification settings - Fork 53
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
Advanced Trip Form #1249
Advanced Trip Form #1249
Conversation
…pplanner/otp-react-redux into advanced-trip-form-panel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was the mobile view of Advanced Settings supposed to be tiny?
@@ -22,20 +45,28 @@ interface Props { | |||
|
|||
class BatchSearchScreen extends Component<Props> { | |||
state = { | |||
planTripClicked: false | |||
closeAdvancedSettingsWithDelay: false, | |||
planTripClicked: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird, yes, it should expand to full screen when opened. Where are you seeing this? Just tested this branch on safari, chrome, and firefox on mobile and I can't reproduce this. Is it showing up in a certain context?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that on Firefox devtools, Firefox on Android, Chrome devtools and Chromium on Android.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks much better, thank you for the changes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just got this thought after looking at the location selector for the mobile view, any of the following could be done in another PR... It seems like this could be a dialog, at least in the mobile view, what do you think? That way, the Advanced Trip Options panel can be dismissed using the ESC key, and for desktop, the focus would be contained within the dialog.
…planner/otp-react-redux into master-trip-form-update
Description:
Moves mode subsettings into "Advanced" trip panel. Requires opentripplanner/otp-ui#749
PR Checklist: