chore: Select component refactoring - SelectControl - Iteration 5 #16510

geido · 2021-08-30T15:51:04Z

SUMMARY

The goal of this PR is to remove the instances of the older react-select Selects and replace them with the Antdesign Select component. It removes the SelectControl component where possible OR refactors it by mapping the props to minimize the impact.

BEFORE/AFTER FILTER BOX

filterbox_before.mp4

filterbox_after.mp4

BEFORE/AFTER SPATIAL CONTROL

spatialcontrol_before.mp4

spatialcontrol_after.mp4

BEFORE/AFTER ANNOTATION LAYERS

annotation_layers_before.mp4

annotation_layers_after.mp4

BEFORE/AFTER DATASOURCE EDITOR

datasource_editor_before.mp4

datasource_editor_after.mp4

BEFORE/AFTER CONTROLS

controls_before.mp4

controls_after.mp4

TESTING INSTRUCTIONS

Test each component individually and all possible controls

ADDITIONAL INFORMATION

Has associated issue:
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

geido · 2021-08-31T13:41:18Z

superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx

@@ -48,58 +48,27 @@ describe('SelectControl', () => {
    wrapper = shallow(<SelectControl {...defaultProps} />);
  });

-  it('uses Select in onPasteSelect when freeForm=false', () => {


The OnPasteSelect is not used with the new Select component. These tests are included later checking whether allowNewOptions is true or false based on the SelectControl freeForm option.

geido · 2021-08-31T13:41:53Z

superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx

    expect(defaultProps.onChange.calledWith(50)).toBe(true);
  });

-  it('returns all options on select all', () => {


The "Select all" option has been removed as agreed with design and all the related tests have been removed as well

geido · 2021-08-31T13:42:44Z

superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx

@@ -161,16 +130,6 @@ describe('SelectControl', () => {
        );
        expect(wrapper.html()).not.toContain('add something');
      });
-      it('shows numbers of options as a placeholder by default', () => {


The number of options and remaining options has been removed as agreed with design and all the related tests have been removed as well

geido · 2021-08-31T13:43:48Z

superset-frontend/spec/javascripts/explore/components/SelectControl_spec.jsx

-      });
-    });
-
-    it('returns the correct options when freeform is set to true', () => {


This test is not necessary with the new Select component as it is a standard behavior

…e/select-component-refactoring-selectcontrol-iteration-5

codecov · 2021-09-16T16:22:44Z

Codecov Report

Merging #16510 (5c76195) into master (4086bed) will decrease coverage by 0.04%.
The diff coverage is 66.66%.

❗ Current head 5c76195 differs from pull request most recent head 748af87. Consider uploading reports for the commit 748af87 to get more accurate results

@@            Coverage Diff             @@
##           master   #16510      +/-   ##
==========================================
- Coverage   77.00%   76.96%   -0.05%     
==========================================
  Files        1018     1018              
  Lines       54654    54580      -74     
  Branches     7454     7427      -27     
==========================================
- Hits        42086    42005      -81     
- Misses      12324    12328       +4     
- Partials      244      247       +3

Flag	Coverage Δ
javascript	`70.97% <66.66%> (-0.11%)`	⬇️

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

Impacted Files	Coverage Δ
...mponents/controls/AnnotationLayerControl/index.jsx	`8.33% <0.00%> (+0.54%)`	⬆️
...src/explore/components/controls/SpatialControl.jsx	`6.75% <ø> (ø)`
superset-frontend/src/explore/controls.jsx	`29.72% <ø> (+1.15%)`	⬆️
...ontrols/AnnotationLayerControl/AnnotationLayer.jsx	`84.28% <50.00%> (+0.42%)`	⬆️
.../src/explore/components/controls/SelectControl.jsx	`80.00% <68.75%> (-9.39%)`	⬇️
...erset-frontend/src/datasource/DatasourceEditor.jsx	`73.85% <100.00%> (ø)`
...components/controls/FilterBoxItemControl/index.jsx	`73.58% <100.00%> (ø)`
...rontend/src/components/Select/DeprecatedSelect.tsx	`67.64% <0.00%> (-14.71%)`	⬇️
.../explore/components/controls/TextControl/index.tsx	`82.92% <0.00%> (-4.88%)`	⬇️
superset-frontend/src/components/Select/styles.tsx	`81.94% <0.00%> (-2.78%)`	⬇️
... and 2 more

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 4086bed...748af87. Read the comment docs.

geido · 2021-09-17T14:03:26Z

/testenv up

github-actions · 2021-09-17T14:05:30Z

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

...set-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx

…yerControl/AnnotationLayer.test.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>

pkdotson · 2021-09-29T02:35:29Z

/testenv up

github-actions · 2021-09-29T02:39:48Z

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

pkdotson

LGTM! Tested in eph env and everything works great!

villebro

This is amazing work, comparing the before/after was such a visible improvement! 🎉 LGTM!

villebro · 2021-09-29T08:07:18Z

superset-frontend/src/datasource/DatasourceEditor.jsx

+const DATA_TYPES = [
+  { value: 'STRING', label: 'STRING' },
+  { value: 'NUMERIC', label: 'NUMERIC' },
+  { value: 'DATETIME', label: 'DATETIME' },
+  { value: 'BOOLEAN', label: 'BOOLEAN' },
+];


Not for this PR, but this probably shouldn't be a Select component at all, but rather a text field. These should be raw column types coming from the database, hence a typical STRING will be VARCHAR(x), TEXT etc.

github-actions · 2021-10-04T15:25:04Z

Ephemeral environment shutdown and build artifacts deleted.

…ache#16510) * Refactor Select DatasourceEditor * Fire onChange with allowNewOptions * Clean up * Refactor Select in AnnotationLayer * Handle on clear * Update tests * Refactor Select in SpatialControl * Show search * Refactor Select in FilterBox * Remove search where unnecessary * Update SelectControl - WIP * Refactor Controls * Update SelectControl tests * Clean up * Test allowNewOptions false * Use SelectControl AnnotationLayer * Use SelectControl SpatialControl * Clean up * Render custom label * Show search * Implement filterOption * Improve filterOption * Update Cypress * Update Cypress table test * Use value for defaultValue * Merge with latest changes * Reconcile with latest Select changes * Update superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Update superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Revert changes to test * Call onPopoverClear when v value is undefined Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>

geido added 9 commits August 29, 2021 18:11

Refactor Select DatasourceEditor

5e02cfd

Fire onChange with allowNewOptions

8297208

Clean up

0885f6c

Refactor Select in AnnotationLayer

8c3b96b

Handle on clear

16341a7

Update tests

5ed953c

Refactor Select in SpatialControl

67a2332

Show search

baa04dc

Refactor Select in FilterBox

46be115

superset-github-bot bot added the Flexiana label Aug 30, 2021

pull-request-size bot added the size/L label Aug 30, 2021

geido marked this pull request as draft August 30, 2021 15:51

geido added 2 commits August 30, 2021 19:04

Remove search where unnecessary

75e2dcb

Update SelectControl - WIP

c1a867f

pull-request-size bot added size/XL and removed size/L labels Aug 30, 2021

geido added 3 commits August 31, 2021 15:47

Refactor Controls

9b0ec18

Update SelectControl tests

d89fabe

Clean up

39d86c9

geido commented Aug 31, 2021

View reviewed changes

geido added 7 commits August 31, 2021 16:44

Test allowNewOptions false

ffcb26b

Use SelectControl AnnotationLayer

a965366

Use SelectControl SpatialControl

3118a55

Clean up

916fc29

Merge branch 'master' of https://github.com/apache/superset into chor…

a17164b

…e/select-component-refactoring-selectcontrol-iteration-5

Render custom label

e48e428

Show search

738ee0d

Reconcile with latest Select changes

7bfcd93

geido requested review from michael-s-molina, etr2460 and ktmud and removed request for michael-s-molina September 16, 2021 15:58

geido removed the hold:testing! On hold for testing label Sep 16, 2021

michael-s-molina reviewed Sep 20, 2021

View reviewed changes

...set-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx Show resolved Hide resolved

...set-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx Show resolved Hide resolved

geido and others added 3 commits September 21, 2021 14:20

Update superset-frontend/src/explore/components/controls/AnnotationLa…

5a94d56

…yerControl/AnnotationLayer.test.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>

Update superset-frontend/src/explore/components/controls/AnnotationLa…

3a5ee3f

…yerControl/AnnotationLayer.test.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>

Revert changes to test

353ae00

michael-s-molina requested review from pkdotson, suddjian and villebro September 22, 2021 13:13

geido added 2 commits September 23, 2021 15:39

Call onPopoverClear when v value is undefined

21dc30e

Remove unnecessary onClear checks

748af87

pkdotson approved these changes Sep 29, 2021

View reviewed changes

villebro approved these changes Sep 29, 2021

View reviewed changes

geido merged commit 3f0756f into apache:master Oct 4, 2021

geido deleted the chore/select-component-refactoring-selectcontrol-iteration-5 branch October 4, 2021 15:24

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Select component refactoring - SelectControl - Iteration 5 #16510

chore: Select component refactoring - SelectControl - Iteration 5 #16510

geido commented Aug 30, 2021 •

edited

geido Aug 31, 2021

geido Aug 31, 2021

geido Aug 31, 2021

geido Aug 31, 2021

codecov bot commented Sep 16, 2021 •

edited

geido commented Sep 17, 2021

github-actions bot commented Sep 17, 2021

pkdotson commented Sep 29, 2021

github-actions bot commented Sep 29, 2021

pkdotson left a comment

villebro left a comment

villebro Sep 29, 2021

github-actions bot commented Oct 4, 2021

chore: Select component refactoring - SelectControl - Iteration 5 #16510

chore: Select component refactoring - SelectControl - Iteration 5 #16510

Conversation

geido commented Aug 30, 2021 • edited

SUMMARY

BEFORE/AFTER FILTER BOX

BEFORE/AFTER SPATIAL CONTROL

BEFORE/AFTER ANNOTATION LAYERS

BEFORE/AFTER DATASOURCE EDITOR

BEFORE/AFTER CONTROLS

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

geido Aug 31, 2021

Choose a reason for hiding this comment

geido Aug 31, 2021

Choose a reason for hiding this comment

geido Aug 31, 2021

Choose a reason for hiding this comment

geido Aug 31, 2021

Choose a reason for hiding this comment

codecov bot commented Sep 16, 2021 • edited

Codecov Report

geido commented Sep 17, 2021

github-actions bot commented Sep 17, 2021

pkdotson commented Sep 29, 2021

github-actions bot commented Sep 29, 2021

pkdotson left a comment

Choose a reason for hiding this comment

villebro left a comment

Choose a reason for hiding this comment

villebro Sep 29, 2021

Choose a reason for hiding this comment

github-actions bot commented Oct 4, 2021

geido commented Aug 30, 2021 •

edited

codecov bot commented Sep 16, 2021 •

edited