9402 advanced search filter #585
Conversation
smoyte
force-pushed
the
9402_searchbox_filter
branch
from
668da23 to
711b60a
Compare
smoyte
left a comment
smoyte
left a comment
There was a problem hiding this comment.
FYI: I rebased this against develop with the other PR merged.
It took me awhile to figure out why the box always says query 😅. Now it makes perfect sense of course but in future with these PRs can you remind me what is stubbed vs. what real functionality you're adding so I can remember what to be testing for/scrutinizing. It would help me get back into the context. Thanks.
Overall it looks great. A couple small comments inline, plus can we get some space between these buttons please:
Thanks a lot!
| onChange={[MockFunction]} | ||
| placeholder="Advanced search" | ||
| type="text" | ||
| value="query" |
There was a problem hiding this comment.
Shouldn't this be foo? I'm not seeing foo anywhere in this snapshot even though that's the query param...
There was a problem hiding this comment.
OH RIGHT. I guess this snapshot will change once we integrate with the backend.
There was a problem hiding this comment.
Actually this value is coming from the test mock in spec/.../search/utils.js which defines advancedSearchProps. The AdvancedSearchFilter component itself gets props directly, it doesn't care what the actual url querystring is.
|
@cooperka Hmm, it seems there are a lot of CI failures. We will need to figure those out... |
cooperka
force-pushed
the
9402_searchbox_filter
branch
from
495b7c7 to
e37f663
Compare
cooperka
force-pushed
the
9402_searchbox_filter
branch
from
8546281 to
eeee4de
Compare
|
@smoyth tests should finally be passing. There were a lot of weird inconsistencies with the way Ruby talks to React, and also with the way Capybara manipulates the DOM. I squashed the hell out of the commits, hopefully the solution is fairly readable now. Ready for review. |
Integrates the existing search box into the new Filters component.