feat(dropdown): SingleSelect with search filter #903

jandrade · 2020-05-21T00:34:19Z

Summary

Similar to the MultiSelect component (#479), these changes introduce the isFilterable prop to <SingleSelect />.

This allows us to filter options inside the single select component.

Test plan

Open https://5e1bf4b385e3fb0020b7073c-vojuppvbfz.chromatic.com/?path=/story/dropdown-singleselect--custom-styles
Verify that the search input is present and that we can filter options by query.

codecov · 2020-05-21T00:35:39Z

Codecov Report

Merging #903 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #903      +/-   ##
==========================================
+ Coverage   96.16%   96.19%   +0.02%     
==========================================
  Files         139      139              
  Lines        2373     2391      +18     
  Branches      454      457       +3     
==========================================
+ Hits         2282     2300      +18     
  Misses         85       85              
  Partials        6        6

Impacted Files	Coverage Δ
.../wonder-blocks-dropdown/components/multi-select.js	`96.73% <100.00%> (+0.03%)`	⬆️
...wonder-blocks-dropdown/components/single-select.js	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 889c44a...ed62431. Read the comment docs.

kevinbarabash

I'm excited that you're back porting this functionality from webapp to wonder-blocks. At a high-level I think we need to decide if we want this functionality in all of our dropdown types. If we do, it shouldn't be too much work to push the logic down into dropdown-core.js.

Requesting the following changes:

expand the testing to verify the interaction of selection and fitlering
fix what appears to be a missing return in getMenuItems
enable screenshotting on a story with the dropdown open on page load (if possible)

There are some other nits in the comments which I leave to your discretion.

packages/wonder-blocks-dropdown/components/multi-select.stories.js

kevinbarabash · 2020-05-26T17:52:30Z

packages/wonder-blocks-dropdown/components/__tests__/single-select.test.js

+            // Assert
+            expect(wrapper.find(IconButton).exists()).toBe(true);


Asserting on the props of SearchTextInput would be more robust since what SearchTextInput renders may change. Also, it's possible that the OptionItems might change in the future to render an IconButton.

Now that you mention it, I think it makes more sense to move this test inside search-text-input.test.js. The SingleSelect shouldn't worry about these specific details, and we are checking if the search input is present in the previous test.

kevinbarabash · 2020-05-26T17:53:41Z

packages/wonder-blocks-dropdown/components/__tests__/single-select.test.js

+            // Arrange
+            // Arrange


kevinbarabash · 2020-05-26T17:55:37Z

packages/wonder-blocks-dropdown/components/__tests__/single-select.test.js

+            // Assert
+            expect(wrapper).toHaveState({searchText: ""});
+        });
+    });


It would be good to have some tests that cover the interaction between filtering and selection. If the selected item doesn't match the search filter doesn't it still appear? If so, where does it appear? If not, how doesn't navigation from the selected item work?

@jandrade friendly ping.

Oh I forgot this comment. Thanks for the reminder.

kevinbarabash · 2020-05-26T17:57:12Z

packages/wonder-blocks-dropdown/components/single-select.stories.js

+    },
+});
+
+export const customStyles = () => <SingleSelectWithCustomStyles />;


The name of this story should have something about filter/search in the title.

Seeing, this I just realized that if we allow hooks in wonder-block at least, we wouldn't need a class component here.

Aside: it would be nice if we could reuse certain stories in the docs instead of having to duplicate the same code in two places.

Good call, I'll change that. About hooks, I thought about that, but I'd prefer to keep using classes as examples here in case folks using Webapp can refer to these snippets in a way they can directly use in Webapp.

I'll gladly replace these examples once we fully enable hooks in Webapp.

Re: aside: I'll be reusing those stories in my next PR (Storybook Docs)... wait for it :)

kevinbarabash · 2020-05-26T20:19:07Z

packages/wonder-blocks-dropdown/components/__tests__/search-text-input.test.js

+import * as React from "react";
+import {shallow} from "enzyme";
+
+import SearchTextInput from "../search-text-input.js";


naming suggestion: SearchField. You using this terminology elsewhere in this PR, but beyond that I think it generalizes better, e.g.

TextField

PasswordField

SearchField

I agree, but I'd prefer to keep it that way to avoid modifying the functionality we already have in place for MultiSelect. I wanted to touch as less as I could here to avoid doing any unnecessary refactors on the original implementation. As you mentioned I'll add a new ticket/TODO to work on this in the future.

kevinbarabash · 2020-05-26T20:22:31Z

packages/wonder-blocks-dropdown/components/__tests__/search-text-input.test.js

+        const wrapper = shallow(
+            <SearchTextInput
+                searchText="query"
+                testId="search-text-input"


nit: is the testId necessary?

kevinbarabash · 2020-05-26T20:23:04Z

packages/wonder-blocks-dropdown/components/__tests__/search-text-input.test.js

+describe("SearchTextInput", () => {
+    it("text input container should be focused when focusing on the input", () => {
+        // Arrange
+        const wrapper = shallow(


Nice use of shallow. 🎉

kevinbarabash · 2020-05-26T20:24:12Z

packages/wonder-blocks-dropdown/__tests__/generated-snapshot.test.js

@@ -669,7 +669,7 @@ describe("wonder-blocks-dropdown", () => {
            darkBackgroundWrapper: {
                flexDirection: "row",
                justifyContent: "flex-end",
-                backgroundColor: Color.darkBlue,
+                background: Color.darkBlue,


Why the switch to background?

For some reason, the tests were throwing a warning about using background and backgroundColor, but I don't remember what was, and now I don't see it, so I 'll revert back to bgColor.

kevinbarabash · 2020-05-26T20:25:45Z

packages/wonder-blocks-dropdown/components/single-select.js

+    /**
+     * The text input to filter the items by their label. Defaults to an empty
+     * string.
+     */
+    searchText: string,


Is this something that we'll want to have on MultiSelect and ActionMenu as well? If so, it might make sense to push some of the logic you added to this component into dropdown-core.js.

I see now that searchText is already part of DropdownCore.

somewhatabstract · 2020-05-26T22:53:43Z

packages/wonder-blocks-dropdown/components/single-select.js

+     * When this is true, the dropdown body shows a search text input at the
+     * top. The items will be filtered by the input.
+     */
+    isFilterable?: boolean,


I wonder if we should allow the filter algorithm to be overridden?

There are some really handy filtering rules besides "contains case insensitive match" such as the filtering rules that VSCode has for finding files, like matching starts of words. For example, ABC would match "Always Be Closing".

@lillialexis wrote a pretty awesome smart matching algorithm for our github CLI tools would be nice to use.

I did write a filter function that does exactly that @somewhatabstract! https://github.com/Khan/our-lovely-cli/blob/develop/cli/pull-request/smart-suggest.js

jandrade · 2020-06-15T18:25:03Z

.github/workflows/node-ci.yml

@@ -13,7 +13,7 @@ jobs:
      - name: Use Node.js 12.x
        uses: actions/setup-node@v1
        with:
-          node-version: 12.x
+          node-version: 12.16.x


I changed this b/c storybook/chromatic was failing to build: storybookjs/storybook#10477 (comment)

Hopefully they'll fix the underlying issue in the future.

jandrade · 2020-06-15T18:25:34Z

.github/workflows/node-ci.yml

@@ -35,5 +35,5 @@ jobs:
      - uses: chromaui/action@v1
        with:
          token: ${{ secrets.GITHUB_TOKEN }}
-          appCode: ${{ secrets.CHROMATIC_APP_CODE }}
+          projectToken: ${{ secrets.CHROMATIC_APP_CODE }}


Changed it to remove a Chromatic warning.

kevinbarabash

LGTM. There are maybe a few additional tests that could be added, see my friendly ping, but nothing blocking. Thanks for adding filtering to SingleSelect. ❤️

jandrade added 2 commits May 20, 2020 20:24

feat(dropdown): Add isFilterable to SingleSelect

92cca8e

feat(dropdown): Add SingleSelect story

8b889a1

jandrade requested a review from a team May 21, 2020 00:34

jandrade added 2 commits May 21, 2020 16:07

feat(dropdown): Revert xdescribe

bf7de42

feat(dropdown): Fix story

379e410

kevinbarabash requested changes May 26, 2020

View reviewed changes

somewhatabstract reviewed May 26, 2020

View reviewed changes

feat(dropdown): Address PR comments

e1a6b56

jandrade requested a review from kevinbarabash June 15, 2020 15:05

jandrade added 3 commits June 15, 2020 13:14

chore(chromatic): Upgrade to v5

91c25fc

chore(chromatic): Update GH action

fe08090

chore(chromatic): Lock Node version to 12.16.x

879efd6

jandrade commented Jun 15, 2020

View reviewed changes

jandrade added 3 commits June 16, 2020 12:28

feat(dropdown): Fix stories to show the opened variant

a5b6043

feat(dropdown): Fix MultiSelect story to show the opened variant

9d40c25

merge origin/master into dropdown-single-filter

ed62431

kevinbarabash approved these changes Jun 16, 2020

View reviewed changes

jandrade merged commit 28063d8 into master Jun 17, 2020

jandrade deleted the dropdown-single-filter branch June 17, 2020 19:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dropdown): SingleSelect with search filter #903

feat(dropdown): SingleSelect with search filter #903

jandrade commented May 21, 2020 •

edited

codecov bot commented May 21, 2020 •

edited

kevinbarabash left a comment

kevinbarabash May 26, 2020

jandrade May 26, 2020 •

edited

kevinbarabash May 26, 2020

kevinbarabash May 26, 2020

kevinbarabash Jun 16, 2020

jandrade Jun 16, 2020

kevinbarabash May 26, 2020

jandrade May 26, 2020

kevinbarabash May 26, 2020

jandrade May 26, 2020

kevinbarabash May 26, 2020

kevinbarabash May 26, 2020

kevinbarabash May 26, 2020

jandrade May 26, 2020

kevinbarabash May 26, 2020

kevinbarabash Jun 16, 2020

somewhatabstract May 26, 2020

kevinbarabash May 27, 2020

lillialexis May 27, 2020

jandrade Jun 15, 2020

kevinbarabash Jun 16, 2020

jandrade Jun 15, 2020

kevinbarabash left a comment

		// Assert
		expect(wrapper.find(IconButton).exists()).toBe(true);

feat(dropdown): SingleSelect with search filter #903

feat(dropdown): SingleSelect with search filter #903

Conversation

jandrade commented May 21, 2020 • edited

Summary

Test plan

codecov bot commented May 21, 2020 • edited

Codecov Report

kevinbarabash left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jandrade May 26, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinbarabash left a comment

Choose a reason for hiding this comment

jandrade commented May 21, 2020 •

edited

codecov bot commented May 21, 2020 •

edited

jandrade May 26, 2020 •

edited