-
Notifications
You must be signed in to change notification settings - Fork 34
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(trip-form): handle date/time state internally #740
base: master
Are you sure you want to change the base?
feat(trip-form): handle date/time state internally #740
Conversation
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.
Annoying, but seems to work. One important question before I can approve
departArrive: "NOW", | ||
departArrive: "DEPART", |
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.
Do implementers have to change this as well? If so this would be a breaking change no?
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.
No they don't, this is just for the story because when it's set to NOW you don't get to see the actual date time selector. Ideally this would be a control in the story but the way we've set up the story here makes that difficult, should probably redo that at some point.
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.
Ah I see. Ok looks good!
@miles-grant-ibigroup can we remove the legacy code? I didn't touch it but actually I think it would need to be changed here as well if we're not going to remove it. Can you check into browser support? https://caniuse.com/input-datetime |
I checked feel free to remove |
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.
Still feels scary... but onwards and upwards!
This internal state handling will help us deal with some state problems when storing the state in the URL in OTP-RR.