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

feat(trip-form): handle date/time state internally #740

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daniel-heppner-ibigroup
Copy link
Contributor

This internal state handling will help us deal with some state problems when storing the state in the URL in OTP-RR.

Copy link
Collaborator

@miles-grant-ibigroup miles-grant-ibigroup left a 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",
Copy link
Collaborator

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?

Copy link
Contributor Author

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.

Copy link
Collaborator

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!

@daniel-heppner-ibigroup
Copy link
Contributor Author

@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

@miles-grant-ibigroup
Copy link
Collaborator

@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

Copy link
Collaborator

@miles-grant-ibigroup miles-grant-ibigroup left a 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!

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.

None yet

2 participants