Arvados » Arvados Workbench 2

I've also seen this misbehavior on the Users UI's search field.

New version: https://dev.arvados.org/projects/arvados/repository/arvados-workbench2/revisions/235d9456eb6679611af96383f9dfcadc3462a7da
Test run: developer-tests-workbench2: #953
Branch: 19311-Project-Search-field-auto-clears

Added inital check before search field clear

• The reported issue on the projects view seems to be working well now, thanks.
• OTOH, I've checked how the search input behaves on the collection's file browser and the previous problematic behavior persists. (try a collection with a subdirectory so that the search input is in both panels of the file browser)
• I'm worried about that global selfClearPropState variable that was added at src/components/search-input/search-input.tsx, I'm thinking that it could create issues difficult to debug, can you make the component's state completely isolated? If more than one instance of the component is in use, that global would be a shared state between both input fields, and that's not what we want, right?

New version: https://dev.arvados.org/projects/arvados/repository/arvados-workbench2/revisions/e4f350462118a86831f92eb375c2bc89cbf85920
Test run: developer-tests-workbench2: #962
Branch: 19311-Project-Search-field-auto-clears

Changed component type to a functional approach

Commit arvados-workbench2|e4f3504 seems to be just a main merge, are you sure all the pending work was checked in and pushed?

New version: https://dev.arvados.org/projects/arvados/repository/arvados-workbench2/revisions/a9a30db7ec9505450da696ccf660edc20fe2ac0b
Test run: developer-tests-workbench2: #969
Branch: 19311-Project-Search-field-auto-clears

Changed component type to a functional approach

• Manual tests show correct behavior.
• There's some commented out code that could be just deleted.
• On the test file src/components/search-input/search-input.test.tsx I'm not sure the modified test is correctly exercising the behavior, for example I tried commenting out line 101 and re-ran the test, still getting a success. I've even tried passing setProps({ selfClearProp: 'abc' }) on line 101 and it also succeeds.

New version: https://dev.arvados.org/projects/arvados/repository/arvados-workbench2/revisions/cbe483113381e8c22b28a042479f5b7374a84744
Test run: developer-tests-workbench2: #970
Branch: 19311-Project-Search-field-auto-clears

Removed commented code, made tests clear

I think tests like this should usually exercise both ways of a behavior: when it should and shouldn't happen.
I've added a commit (arvados-workbench2|6887b04c) with the following:

diff --git a/src/components/search-input/search-input.test.tsx b/src/components/search-input/search-input.test.tsx
index ff1d30c4..ba70f752 100644
--- a/src/components/search-input/search-input.test.tsx
+++ b/src/components/search-input/search-input.test.tsx
@@ -104,6 +104,11 @@ describe("<SearchInput />", () => {
             expect(onSearch).toBeCalledWith("");
             expect(onSearch).toHaveBeenCalledTimes(1);

+            // component should not clear on same selfClearProp
+            searchInput.setProps({ selfClearProp: 'abc' });
+            jest.runTimersToTime(1000);
+            expect(onSearch).toHaveBeenCalledTimes(1);
+
             // component should clear on selfClearProp change
             searchInput.setProps({ selfClearProp: '111' });
             jest.runTimersToTime(1000);

...so that we make sure the cleaning is done only when it should.

With that, it LGTM.