add content filtering to the website content listing page #1046
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1046 +/- ##
==========================================
+ Coverage 91.10% 91.12% +0.02%
==========================================
Files 200 201 +1
Lines 7652 7670 +18
Branches 1333 1333
==========================================
+ Hits 6971 6989 +18
Misses 596 596
Partials 85 85
Continue to review full report at Codecov.
|
alicewriteswrongs
force-pushed
the
ap/content-filtering
branch
from
690637e to
12d5225
Compare
| await wait(800) | ||
| wrapper.update() | ||
| expect(wrapper.find(".site-search-input").prop("value")).toBe( | ||
| "my-search-string" | ||
| ) |
There was a problem hiding this comment.
Question / Requested Change: What's the goal of this test? I think it might not be doing what is expected.
Doesn't the input update immediately, but the history / url bar has debounced updating? I tried this test out and the assertion passes without the wait(800).
Is the goal to test the debouncing history thing? If yes, two suggestions:
-
I think
jest.spyOn(helper.browserHistory, 'push')should let us spy on the url bar... but maybe theres a better way. Unsure. -
Instead of using
wait, you could usejest.useFakeTimersandjest.advanceFakeTimers... :timelord:Although, I imagine
waitisn't a significant contributor to our test runtime 🤷
Anyway, requested change is: remove the wait(800) if the test is the way it is is supposed to be, or make it fail without the wait(800).
There was a problem hiding this comment.
I had a heck of a time with this test! I'll give spyOn a try, but it was the strangest thing - although I had an almost identical test to the corresponding one for the SitesDashboard, for some reason the calls to history.push in the hook weren't changing any kind of history anywhere that I could pull out and assert, so I got a little frustrated and then just asserted that the new search string was present as the value prop on the input. I'll give it another go today :)
There was a problem hiding this comment.
ok spyOn worked, but still if I console.log(helper.browserHistory.location) the .search prop is ""... 🤷♀️
| const filterInput = wrapper.find(".site-search-input") | ||
| const event = { | ||
| // eslint-disable-next-line @typescript-eslint/no-empty-function | ||
| preventDefault() {}, |
There was a problem hiding this comment.
Random question: I noticed useTextInputState in your hook, so I went to go look at it... do you know why we call preventDefault in useTextInputState? When is preventing default on a text input important?
There was a problem hiding this comment.
haha, no idea - it probably just got put in by default
|
why just "Pages" ? |
|
@Ferdi it's not just present on pages, I just neglected to screenshot them :) Here's what it looks like elsewhere:
|
alicewriteswrongs
force-pushed
the
ap/content-filtering
branch
from
a6c8706 to
f1a47db
Compare
Pre-Flight checklist
What are the relevant tickets?
for #1020
What's this PR do?
this PR implements a basic sort UI on the content listing pages. I pulled out the logic I wrote for the same type of sort on the
/sitespage into a hook and then added it to the content page, so it works the same way.How should this be manually tested?
go to a content page like
/sites/$SITENAME/type/pageand try it out!Screenshots (if appropriate)