Feature #17579
closed
Clear table filter when changing the project
Added by Daniel Kutyła over 3 years ago.
Updated almost 3 years ago.
Estimated time:
(Total: 0.00 h)
Release relationship:
Auto
Description
Common usage pattern:
User is filtering the content of a project for subprojects by a specific search pattern
User is selecting the found subproject to see what is inside
The project is appears empty because the filter entered before is still set and not cleared when changing the route
🚨 PANIC🚨
Desired behaviour: The "Search" field should be cleared when changing to a different route. This should also be done for other filters in the table as, imho, switching the project switches the context and any applied filter makes no sense anymore
- Status changed from New to In Progress
- Assigned To set to Daniel Kutyła
- Target version set to 2021-12-08 sprint
- Priority changed from Normal to Urgent
Some comments:
- There were a couple of failed tests on Jenkins.
- At the new cypress test, I believe you can avoid creating the second project, as it isn't being used. Could you also add a test for the collection file browser?
- I think it would be convenient to add a unit test with this new feature on
src/components/search-input/search-input.test.tsx
. With the cypress tests we're confirming that the SearchInput
users are correctly passing the new prop to it, but a test for the new feature itself would be useful too, wdyt?
- Is the file
src/components/collection-panel-files/collection-panel-files2.tsx
the one holding the old file browser? I don't believe we should keep it as a duplicate, adding maintenance burden without getting any benefit from it. If we need to get it back for some reason (which I seriously doubt), we could just use git's features to bring it back from the past, do you agree?
- At file
src/components/data-explorer/data-explorer.tsx
Line 89: Does it really need to be added in the local state? It seems that later on it isn't retrieved from it, or am I missing something? (as a quick test I've just commented those lines and the search input clearing still works)
- Target version changed from 2021-12-08 sprint to 2022-01-05 sprint
The updates look good, but I have a couple of questions regarding the SearchInput
component unit test:
- At file
src/components/search-input/search-input.test.tsx
:
- Line 99: I think it would be convenient to also add an opposite case so that it's really clear what the behavior of the component should be with and without the
selfClearProp
prop, do you agree?
- By reading the new test, it's not 100% clear to me what's trying to prove, for example if I change the
setProps
call at line 101 to pass an 'abc'
argument instead of 'aaa'
, the test still succeeds, but OTOH if I comment out line 101 entirely, the test fails. I think both cases should be equivalent so maybe the test should be more complete or the code being tested doesn't behave as expected?
Everything work as expected the selfClearProp is a trigger for the value clear and it should point to the unique element of the component that is using for example for the project it will be project uuid, so when we change the project the search input will clear itself as selfClearProp will change
I don't think the better naming of the test addresses neither one of my comments, but if you believe they don't apply, please go ahead and merge.
- Status changed from In Progress to Resolved
- % Done changed from 0 to 100
Also available in: Atom
PDF