Feature #17579
closedClear table filter when changing the project
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
Updated by Daniel Kutyła almost 3 years ago
- Target version set to 2021-12-08 sprint
- Assigned To set to Daniel Kutyła
- Status changed from New to In Progress
Updated by Daniel Kutyła almost 3 years ago
- Priority changed from Normal to Urgent
Updated by Daniel Kutyła almost 3 years ago
New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/9ec32d73b78aeb92b842aa8cf08bc859e084dcb1
Test run: developer-tests-workbench2: #525
Branch: 17579-Clear-table-filter-when-changing-the-project
Search input field should now be cleared
Updated by Lucas Di Pentima almost 3 years ago
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 theSearchInput
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)
Updated by Daniel Kutyła almost 3 years ago
New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/aac3dfbed3d02cc6a2240add62439ee0dda3f335
Test run: developer-tests-workbench2: #530
Branch: 17579-Clear-table-filter-when-changing-the-project
Fixed tests, cleaned file structure
Updated by Peter Amstutz almost 3 years ago
- Target version changed from 2021-12-08 sprint to 2022-01-05 sprint
Updated by Daniel Kutyła almost 3 years ago
New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/d147177d6a86c45e41e921f119aa87609f4b89e9
Test run: developer-tests-workbench2: #539
Branch: 17579-Clear-table-filter-when-changing-the-project
Fixed tests
Updated by Lucas Di Pentima almost 3 years ago
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?
- 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
Updated by Daniel Kutyła almost 3 years ago
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
Updated by Daniel Kutyła almost 3 years ago
New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/17dfef9d2440ddbac9cd8eac0ec8be721b42cc09
Test run: developer-tests-workbench2: #539
Branch: 17579-Clear-table-filter-when-changing-the-project
I added better description for the test
Updated by Lucas Di Pentima almost 3 years ago
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.
Updated by Daniel Kutyła almost 3 years ago
- % Done changed from 0 to 100
- Status changed from In Progress to Resolved
Applied in changeset arvados-workbench-2:arvados-workbench2|fc84a3f3932af503d3afd04a58af52270c8fc3b6.