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

Add a date selector to the search function #494

Merged
merged 28 commits into from
Oct 4, 2023

Conversation

rleed
Copy link
Contributor

@rleed rleed commented Sep 14, 2023

I'm opening this in draft state (pertaining to issue #410) because I've gotten stuck and I want to ask if someone has insight on what I'm missing.

I've added the date picker to the UI in this PR, and it works fine up to and including updating the values object and the query parameters in the location bar. However, when clicking the search button to actually perform the search, the values revert back to what seem to be the initial values, and I do not understand why or where those are coming from.

Does anyone have insight on why that is happening?

@huumn
Copy link
Member

huumn commented Sep 14, 2023

It's because the search button triggers onSubmit which calls search with the Form values. You are storing your date range in the search component's state and the values sent for onSubmit come from the Form's state.

Make sense?

@rleed
Copy link
Contributor Author

rleed commented Sep 14, 2023

Yeah, that makes sense. I was thinking formik.values took care of it, but now I see I need to use helpers.setValue as well.

@rleed rleed force-pushed the date-selector branch 5 times, most recently from b47cfc4 to 99c6aae Compare September 26, 2023 12:21
@rleed rleed marked this pull request as ready for review September 26, 2023 12:24
@huumn
Copy link
Member

huumn commented Sep 30, 2023

The code for this looks simple and good but the UI is too bad on mobile to ship.

Going to drop in draft until you or I can fix that.

@huumn huumn marked this pull request as draft September 30, 2023 20:27
@rleed
Copy link
Contributor Author

rleed commented Oct 4, 2023

Updated to wrap (only) the date selector on mobile.

image

@rleed rleed marked this pull request as ready for review October 4, 2023 14:33
@huumn huumn merged commit 247744a into stackernews:master Oct 4, 2023
1 check passed
@rleed rleed deleted the date-selector branch October 15, 2023 12:30
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.

2 participants