-
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
feat: date time use url storage #1208
base: dev
Are you sure you want to change the base?
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.
Working well! Just a few small tweaks
*/ | ||
const _onSettingsUpdate = useCallback( | ||
(params: any) => { | ||
console.log('setting') |
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.
Whoops!
const _onSettingsUpdate = useCallback( | ||
(params: any) => { | ||
console.log('setting') | ||
setQueryParam({ queryParamData: params, ...params }) |
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.
Are we really inserting everything twice? That's so annoying
date: urlSearchParams.get('date'), | ||
departArrive: urlSearchParams.get('departArrive'), | ||
time: urlSearchParams.get('time') |
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.
This is simple enough I don't mind the repetition
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.
Actually before I approve I'm a bit nervous about the Percy tests. why did their date change
Description:
This is setting us up for future changes to the date time selector. We need to adapt it to use the URL for the source of truth rather than Redux.