feat: Select all for synchronous select

[cccs-RyanK]

SUMMARY

This PR adds a 'select all' functionality to the synchronous Select component. This functionality has been requested by a large number of users and contributors. The select component has been previously split into sync and async components to simplify the code. The sync select component is the more widely used component, and a select all for the sync case is much easier to implement. This PR simply adds the option for selectAllEnabled only when the select component is in multi-select mode. The option is by default set to false. This is part of the greater task to rework the select component for the select all functionality outlined by @michael-s-molina in this PR: https://github.com/apache/superset/pull/20143

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

In storybook, go to the select component -> interactive select and set the mode to multi and selectAllEnabled to True

ADDITIONAL INFORMATION

• [x] Has associated issue: https://github.com/apache/superset/discussions/18247#discussioncomment-2467080
• [ ] Required feature flags:
• [x] 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

[codecov[bot]]

Codecov Report

Merging #22084 (0d66466) into master (1a0de49) will decrease coverage by 0.10%. The diff coverage is 85.33%.

@@            Coverage Diff             @@
##           master   #22084      +/-   ##
==========================================
- Coverage   67.09%   66.99%   -0.10%     
==========================================
  Files        1869     1876       +7     
  Lines       71597    71814     +217     
  Branches     7821     7879      +58     
==========================================
+ Hits        48035    48110      +75     
- Misses      21534    21675     +141     
- Partials     2028     2029       +1     

Flag	Coverage Δ
javascript	53.79% <85.33%> (-0.12%)	:arrow_down:

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

Impacted Files	Coverage Δ
...set-frontend/src/components/Select/AsyncSelect.tsx	88.46% <ø> (-0.07%)	:arrow_down:
...c/filters/components/Select/SelectFilterPlugin.tsx	62.82% <ø> (-0.48%)	:arrow_down:
superset-frontend/src/components/Select/styles.tsx	80.76% <66.66%> (-1.84%)	:arrow_down:
superset-frontend/src/components/Select/Select.tsx	91.61% <84.61%> (-1.53%)	:arrow_down:
...erset-frontend/src/components/Select/CustomTag.tsx	75.00% <100.00%> (+3.57%)	:arrow_up:
superset-frontend/src/components/Select/utils.tsx	82.14% <100.00%> (+0.66%)	:arrow_up:
superset-frontend/src/components/Modal/Modal.tsx	85.89% <0.00%> (-3.30%)	:arrow_down:
...plugin-chart-echarts/src/Treemap/transformProps.ts	50.87% <0.00%> (-1.37%)	:arrow_down:
superset-frontend/src/GlobalStyles.tsx	0.00% <0.00%> (ø)
.../plugins/plugin-chart-echarts/src/Treemap/types.ts	100.00% <0.00%> (ø)
... and 10 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[kgabryje]

/testenv up

[github-actions[bot]]

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

[EugeneTorap]

Hi @cccs-RyanK. Thank you for this PR! Can we try to use 'Select All' in columns, table viz? Or it'll be next PR?

[michael-s-molina]

When allowNewOptions is enabled, if you add a new option and remove it, "Select All" will be unchecked.

https://user-images.githubusercontent.com/70410625/207110358-091b7ad9-591b-44a5-a0ba-0891756b56ba.mov

[michael-s-molina]

When fixing the reported issues, can you please add the following test cases?

• +N tag does not count the "Select All" option
• does not show +N tag when "Select All" is checked
• "Select All" is checked when unchecking an added option and all the other options are selected

[geido]

Not sure if this was discussed before, but I disagree with the choice of showing the number of items selected when "Select all" is checked and the plain items with the +X when the "Select all" is unchecked. I tend to believe we should always be showing the plain items that fit and the +X which is the default behaviour independently of whether the "Select all" option is checked to avoid the sudden change shown in the video and for consistency reasons

https://user-images.githubusercontent.com/60598000/207359271-47b47db1-f853-4362-b2c4-7170152b7931.mp4

When clearing the "Select all" from the options, the dropdown shows up. I think it shouldn't as the user intends to clear the selection and not to choose other options necessarily. Additionally, if you look at the video you will see that the dropdown is not positioned vertically in the correct way for a moment

https://user-images.githubusercontent.com/60598000/207360266-810a2b6a-9c6f-4033-b3ce-dc10085b93ae.mp4

When allowNewOptions is enabled If you create multiple options and uncheck "Select all", all of the newly created options will be erased. This should not be the case

https://user-images.githubusercontent.com/60598000/207361806-5243f9b0-9699-4509-8a7f-7b01fa97b497.mp4

[michael-s-molina]

Not sure if this was discussed before, but I disagree with the choice of showing the number of items selected when "Select all" is checked and the plain items with the +X when the "Select all" is unchecked. I tend to believe we should always be showing the plain items that fit and the +X which is the default behaviour independently of whether the "Select all" option is checked to avoid the sudden change shown in the video and for consistency reasons

I agree with @geido on this one. It will make it more consistent. You can modify my suggested test cases.

[cccs-RyanK]

When allowNewOptions is enabled If you create multiple options and uncheck "Select all", all of the newly created options will be erased. This should not be the case Select.-.Interactive.Select.Storybook.3.mp4

Thanks for the feedback @geido ! Made the suggested changes from your first two points. As for the last point you made about the new options being erased when they are deselected, I do think that this behaviour is consistent with how the component currently works. For example, before the select-all changes if you add new values by themselves and then deselect them, they are erased from the options. My thoughts are that deselecting-all should then also remove all the new options. Please let me know if that makes sense to you.

[EugeneTorap]

Hi @cccs-RyanK! This PR is important for us. Any news when we can merge it?

[villebro]

FYI @cccs-RyanK @EugeneTorap this is on my priority todo list, and I'm hoping to give it a proper review+test pass this weekend.

[michael-s-molina]

Thanks for the feedback @geido ! Made the suggested changes from your first two points. As for the last point you made about the new options being erased when they are deselected, I do think that this behaviour is consistent with how the component currently works. For example, before the select-all changes if you add new values by themselves and then deselect them, they are erased from the options. My thoughts are that deselecting-all should then also remove all the new options. Please let me know if that makes sense to you.

@cccs-RyanK @geido I think we should disassociate the uncheck and remove actions on new items. We could have an X icon to remove new items. I can think of a scenario where a user is adding filters to a chart and after seeing the results, the user can check/uncheck the added filters to test different combinations. Making the remove action more intentional will improve UX. Let's tackle this in a separate PR. For now, I'm fine with removing all items when Select All is unchecked.

[michael-s-molina]

When working in oneLine mode, it's counting the Select All option.

https://user-images.githubusercontent.com/70410625/211862341-565316ce-bffa-42bc-8a99-f072d5908782.mov

[github-actions[bot]]

Ephemeral environment shutdown and build artifacts deleted.

[john-bodley]

@cccs-RyanK and @michael-s-molina I was wondering whether this feature could be configurable, especially as it pertains to SQL Lab. For example at Airbnb we have namespaces which house thousands of tables/views (see attachment) and if the user accidentally chooses the Select All option` SQL Lab will implode.

It's unclear if this option really has any value in the context of SQL Lab, i.e., I'm struggling to foresee a scenario when one would ever needs to see the schema for all the tables/views within a namespace as opposed to selecting the subset to entities they're interested in.

[michael-s-molina]

I think it makes sense to disable the Select All option in that context.

[villebro]

I agree with @michael-s-molina , let's disable it in that particular context.