feat: remove loading indicator when typing in select #18799

ktmud · 2022-02-17T22:41:23Z

SUMMARY

Remove the loading indicator in select option menus when users are typing and (some) options are already loaded. The loading indicator showing up as users type was added in #16531, while tackling the issue of the inaccurate "No data" message when data is still loading. But we shouldn't show "Loading..." when there are already filtered options available. Removing this unnecessary loading indicator makes the control feels faster.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before

The "Loading..." option shows up as soon as users type, even when all options are loaded:

After

No "Loading..." option when users start to type, but there is a loading indicator in the input and all filtered options will show up after actual loading is finished:

When all options are loaded for a specific search string, there is no loading indicator at all and no excessive requests will be fired.

TESTING INSTRUCTIONS

Run npm run storybook in superset-frontend
Test it in the storybook: http://localhost:6006/?path=/story/select--async-select
1. Try search for something in and not in the first page. E.g. "ale", "sandro"
2. "No data" should not show up if there is a match in loaded options or the next page is still loading
3. "Loading..." should not show up if there is already a matched option

ADDITIONAL INFORMATION

Has associated issue: fix: Adds a loading message when needed in the Select component #16531 chore: Improves the Select component to avoid additional queries when all values have been loaded #16712
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

superset-frontend/src/components/Select/Select.tsx

codecov · 2022-02-22T18:53:46Z

Codecov Report

Merging #18799 (e4ae742) into master (bd63a1b) will decrease coverage by 0.00%.
The diff coverage is 84.21%.

@@            Coverage Diff             @@
##           master   #18799      +/-   ##
==========================================
- Coverage   66.58%   66.58%   -0.01%     
==========================================
  Files        1641     1641              
  Lines       63533    63538       +5     
  Branches     6424     6428       +4     
==========================================
+ Hits        42303    42305       +2     
- Misses      19551    19553       +2     
- Partials     1679     1680       +1

Flag	Coverage Δ
javascript	`51.38% <84.21%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset-frontend/src/components/Select/utils.ts	`59.09% <66.66%> (+1.19%)`	⬆️
superset-frontend/src/components/Select/Select.tsx	`86.69% <85.71%> (-0.67%)`	⬇️

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 bd63a1b...e4ae742. Read the comment docs.

michael-s-molina · 2022-02-23T17:00:24Z

@ktmud I found some problems while testing.

The loading indicator is different when we open the select vs when we search for something.

Screen.Recording.2022-02-23.at.1.44.25.PM.mov

The loading spinner is showing after searching and clicking outside the Select. When fetchOnlyOnSearch is true it never stops spinning.

Screen.Recording.2022-02-23.at.1.55.35.PM.mov

ktmud · 2022-02-23T19:41:45Z

@michael-s-molina Thanks for testing! I've fixed both issues.

michael-s-molina · 2022-02-25T12:07:35Z

@michael-s-molina Thanks for testing! I've fixed both issues.

@ktmud I just tested it again and the bugs are still happening. Did you forget to commit something?

I also noticed that you removed the debounce from handleOnSearch which is triggering this expensive calculation for every typed key.

Screen.Recording.2022-02-25.at.9.03.22.AM.mov

ktmud · 2022-02-28T19:02:45Z

@michael-s-molina are you sure you pulled the latest changes? The bugs are fixed for me.

debounce is removed for handleOnSearch but the actual expensive operations are still debounced as in debouncedHandleSearch.

michael-s-molina · 2022-02-28T19:19:38Z

debounce is removed for handleOnSearch but the actual expensive operations are still debounced as in debouncedHandleSearch.

This part is not debounced:

if (allowNewOptions && isSingleMode) {
    const newOption = searchValue &&
      !hasOption(searchValue, selectOptions) && {
        label: searchValue,
        value: searchValue,
        isNewOption: true,
      };
    const cleanSelectOptions = selectOptions.filter(
      opt => !opt.isNewOption,
    );
    const newOptions = newOption
      ? [newOption, ...cleanSelectOptions]
      : cleanSelectOptions;
    setSelectOptions(newOptions);
  }
  if (isAsync && !allValuesLoaded && loadingEnabled) {
    setIsLoading(search !== searchedValue);
  }

ktmud · 2022-02-28T19:30:20Z

It's not really expensive. The goal of this whole PR is to make the select options update more responsively to user inputs, which is achieved by separating initial filtering and menu state updates with debounced async requests.

michael-s-molina · 2022-02-28T19:50:27Z

@michael-s-molina are you sure you pulled the latest changes? The bugs are fixed for me.

Screen.Recording.2022-02-28.at.4.44.13.PM.mov

michael-s-molina · 2022-02-28T20:14:56Z

It's not really expensive. The goal of this whole PR is to make the select options update more responsively to user inputs, which is achieved by separating initial filtering and menu state updates with debounced async requests.

You can see the loading indicator flicking while you type because it's updating the loading state for every key typed.

Screen.Recording.2022-02-28.at.4.55.19.PM.mov

ktmud · 2022-02-28T22:51:26Z

@michael-s-molina I can reproduce the bug now. Looks like this might need a larger refactoring than I thought it needed. Thanks for testing!

ktmud · 2022-03-01T19:56:00Z

@michael-s-molina This should be ready for re-review.

michael-s-molina

The allowNewOptions is not working as intended. If you add a new option and then search for it then it will disappear from the options.

Screen.Recording.2022-03-02.at.9.38.08.AM.mov

superset-frontend/src/components/Select/Select.tsx

ktmud · 2022-03-02T16:11:12Z

The allowNewOptions is not working as intended. If you add a new option and then search for it then it will disappear from the options.

This should have been fixed

geido · 2022-03-02T17:13:36Z

/testenv up

github-actions · 2022-03-02T17:17:35Z

@geido Ephemeral environment spinning up at http://34.217.29.213:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

michael-s-molina

I haven't found any other problems while testing. Thank you so much @ktmud for improving the user experience of the component and for addressing all suggestions.

@geido was doing some tests with real data so I would kindly request to wait for his approval as well before merging the PR given the critical nature of the component.

michael-s-molina · 2022-03-03T11:43:32Z

superset-frontend/src/components/Select/utils.ts

+    optionsArray.find(x =>
+      typeof x === 'object' ? x.value === value : x === value,
+    ) !== undefined


Suggested change

optionsArray.find(x =>

typeof x === 'object' ? x.value === value : x === value,

) !== undefined

optionsArray.find(option =>

typeof option === 'object' ? option.value === value : option === value,

) !== undefined

In general I prefer to use x in these one liners because sometimes you may want to rename the iterable variable as well.

github-actions · 2022-03-03T17:41:09Z

Ephemeral environment shutdown and build artifacts deleted.

(cherry picked from commit 5a8eb09)

ktmud requested a review from michael-s-molina February 17, 2022 22:41

ktmud requested a review from geido as a code owner February 17, 2022 22:41

pull-request-size bot added the size/S label Feb 17, 2022

ktmud force-pushed the no-loading-when-typing branch from 8feb434 to c5a8317 Compare February 18, 2022 01:32

pull-request-size bot added size/M size/L and removed size/S size/M labels Feb 18, 2022

ktmud force-pushed the no-loading-when-typing branch 3 times, most recently from 09012c8 to 4a8333f Compare February 22, 2022 18:07

ktmud commented Feb 22, 2022

View reviewed changes

superset-frontend/src/components/Select/Select.tsx Show resolved Hide resolved

ktmud force-pushed the no-loading-when-typing branch from 4a8333f to d8f3cce Compare February 23, 2022 19:25

ktmud mentioned this pull request Feb 25, 2022

fix(select): fix typing for customLabel #18952

Closed

9 tasks

ktmud force-pushed the no-loading-when-typing branch from 2f5647b to 1d69a28 Compare March 1, 2022 19:55

ktmud force-pushed the no-loading-when-typing branch from 1d69a28 to 93c02b5 Compare March 1, 2022 23:08

michael-s-molina requested changes Mar 2, 2022

View reviewed changes

superset-frontend/src/components/Select/Select.tsx Show resolved Hide resolved

superset-frontend/src/components/Select/Select.tsx Outdated Show resolved Hide resolved

ktmud added 2 commits March 2, 2022 07:45

feat: remove loading indicator when typing in select

bf13288

Fix tests and more use cases

6957447

ktmud added 8 commits March 2, 2022 07:45

The select component is a neverending landmine

83f3a1d

Use shared constant for debounce

e451841

fix export

1d626ee

Address bugs from code review

d7cce62

Simplify and fix Cypress tests

132ebdf

Improve loading states

3cc313f

More fix

00f60aa

Fix allowNewOption

e4ae742

ktmud force-pushed the no-loading-when-typing branch from 93c02b5 to e4ae742 Compare March 2, 2022 16:10

michael-s-molina approved these changes Mar 3, 2022

View reviewed changes

geido approved these changes Mar 3, 2022

View reviewed changes

ktmud merged commit 5a8eb09 into apache:master Mar 3, 2022

ktmud mentioned this pull request Mar 7, 2022

feat(select): sort exact and startsWith match to first #18856

Merged

9 tasks

villebro added the lts-v1 label Mar 29, 2022

villebro pushed a commit that referenced this pull request Apr 3, 2022

feat: remove loading indicator when typing in select (#18799)

78e85ae

(cherry picked from commit 5a8eb09)

john-bodley deleted the no-loading-when-typing branch June 8, 2022 05:29

mistercrunch added 🍒 1.5.0 🍒 1.5.1 🍒 1.5.2 🍒 1.5.3 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: remove loading indicator when typing in select #18799

feat: remove loading indicator when typing in select #18799

ktmud commented Feb 17, 2022 •

edited

codecov bot commented Feb 22, 2022 •

edited

michael-s-molina commented Feb 23, 2022

ktmud commented Feb 23, 2022

michael-s-molina commented Feb 25, 2022

ktmud commented Feb 28, 2022

michael-s-molina commented Feb 28, 2022

ktmud commented Feb 28, 2022 •

edited

michael-s-molina commented Feb 28, 2022

michael-s-molina commented Feb 28, 2022

ktmud commented Feb 28, 2022

ktmud commented Mar 1, 2022

michael-s-molina left a comment

ktmud commented Mar 2, 2022

geido commented Mar 2, 2022

github-actions bot commented Mar 2, 2022

michael-s-molina left a comment

michael-s-molina Mar 3, 2022

ktmud Mar 3, 2022 •

edited

github-actions bot commented Mar 3, 2022

feat: remove loading indicator when typing in select #18799

feat: remove loading indicator when typing in select #18799

Conversation

ktmud commented Feb 17, 2022 • edited

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before

After

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Feb 22, 2022 • edited

Codecov Report

michael-s-molina commented Feb 23, 2022

ktmud commented Feb 23, 2022

michael-s-molina commented Feb 25, 2022

ktmud commented Feb 28, 2022

michael-s-molina commented Feb 28, 2022

ktmud commented Feb 28, 2022 • edited

michael-s-molina commented Feb 28, 2022

michael-s-molina commented Feb 28, 2022

ktmud commented Feb 28, 2022

ktmud commented Mar 1, 2022

michael-s-molina left a comment

Choose a reason for hiding this comment

ktmud commented Mar 2, 2022

geido commented Mar 2, 2022

github-actions bot commented Mar 2, 2022

michael-s-molina left a comment

Choose a reason for hiding this comment

michael-s-molina Mar 3, 2022

Choose a reason for hiding this comment

ktmud Mar 3, 2022 • edited

Choose a reason for hiding this comment

github-actions bot commented Mar 3, 2022

ktmud commented Feb 17, 2022 •

edited

codecov bot commented Feb 22, 2022 •

edited

ktmud commented Feb 28, 2022 •

edited

ktmud Mar 3, 2022 •

edited