You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by er...@apache.org on 2020/05/27 22:15:54 UTC

[incubator-superset] branch master updated: fix(react-select): FilterBox focus event and adhoc filter popup height (#9933)

This is an automated email from the ASF dual-hosted git repository.

erikrit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 359ea88  fix(react-select): FilterBox focus event and adhoc filter popup height (#9933)
359ea88 is described below

commit 359ea8825d8f1882786852f4bd4015dacf482f89
Author: Jesse Yang <je...@airbnb.com>
AuthorDate: Wed May 27 15:15:32 2020 -0700

    fix(react-select): FilterBox focus event and adhoc filter popup height (#9933)
    
    * fix(react-select): FilterBox focus event and adhoc filter popup height
    
    * Fix flacky cypress test
    
    * Use focus instead of click
---
 .../cypress/integration/dashboard/filter.js        | 30 +++++++++++++--
 ...AdhocFilterEditPopoverSimpleTabContent_spec.jsx |  2 +-
 .../AdhocFilterEditPopoverSimpleTabContent.jsx     |  7 +---
 .../src/visualizations/FilterBox/FilterBox.jsx     | 44 +++++++++++-----------
 4 files changed, 51 insertions(+), 32 deletions(-)

diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/filter.js b/superset-frontend/cypress-base/cypress/integration/dashboard/filter.js
index f739795..3296f81 100644
--- a/superset-frontend/cypress-base/cypress/integration/dashboard/filter.js
+++ b/superset-frontend/cypress-base/cypress/integration/dashboard/filter.js
@@ -53,12 +53,34 @@ export default () =>
     });
 
     it('should apply filter', () => {
-      cy.get('.Select__control')
-        .contains('Select [region]')
-        .click({ force: true });
+      cy.get('.Select__control input[type=text]').first().focus();
+
+      // should open the filter indicator
+      cy.get('.filter-indicator.active')
+        .should('be.visible')
+        .should(nodes => {
+          expect(nodes).to.have.length(9);
+        });
+
+      cy.get('.Select__control input[type=text]').first().blur();
+
+      // should hide the filter indicator
+      cy.get('.filter-indicator')
+        .not('.active')
+        .should(nodes => {
+          expect(nodes).to.have.length(18);
+        });
+
+      cy.get('.Select__control input[type=text]')
+        .first()
+        .focus({ force: true })
+        .type('South Asia', { force: true });
+
+      // type text and <Enter> separately to reduce the change of failing tests
       cy.get('.Select__control input[type=text]')
         .first()
-        .type('South Asia{enter}', { force: true });
+        .focus({ force: true })
+        .type('{enter}', { force: true });
 
       // wait again after applied filters
       cy.wait(aliases.filter(x => x !== getAlias(filterId))).then(requests => {
diff --git a/superset-frontend/spec/javascripts/explore/components/AdhocFilterEditPopoverSimpleTabContent_spec.jsx b/superset-frontend/spec/javascripts/explore/components/AdhocFilterEditPopoverSimpleTabContent_spec.jsx
index 3938abd..f58588e 100644
--- a/superset-frontend/spec/javascripts/explore/components/AdhocFilterEditPopoverSimpleTabContent_spec.jsx
+++ b/superset-frontend/spec/javascripts/explore/components/AdhocFilterEditPopoverSimpleTabContent_spec.jsx
@@ -204,7 +204,7 @@ describe('AdhocFilterEditPopoverSimpleTabContent', () => {
     const { wrapper, onHeightChange } = setup();
 
     wrapper.instance().multiComparatorComponent = {
-      _selectRef: { select: { control: { clientHeight: 57 } } },
+      select: { select: { controlRef: { clientHeight: 57 } } },
     };
     wrapper.instance().handleMultiComparatorInputHeightChange();
 
diff --git a/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx b/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx
index 8d07a42..0b5a02d 100644
--- a/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx
+++ b/superset-frontend/src/explore/components/AdhocFilterEditPopoverSimpleTabContent.jsx
@@ -192,11 +192,8 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon
 
   handleMultiComparatorInputHeightChange() {
     if (this.multiComparatorComponent) {
-      /* eslint-disable no-underscore-dangle */
-      const multiComparatorDOMNode =
-        this.multiComparatorComponent._selectRef &&
-        this.multiComparatorComponent._selectRef.select &&
-        this.multiComparatorComponent._selectRef.select.control;
+      const multiComparatorDOMNode = this.multiComparatorComponent?.select
+        ?.select.controlRef;
       if (multiComparatorDOMNode) {
         if (
           multiComparatorDOMNode.clientHeight !==
diff --git a/superset-frontend/src/visualizations/FilterBox/FilterBox.jsx b/superset-frontend/src/visualizations/FilterBox/FilterBox.jsx
index ad7b306..638ee2c 100644
--- a/superset-frontend/src/visualizations/FilterBox/FilterBox.jsx
+++ b/superset-frontend/src/visualizations/FilterBox/FilterBox.jsx
@@ -100,18 +100,22 @@ class FilterBox extends React.Component {
       // this flag is used by non-instant filter, to make the apply button enabled/disabled
       hasChanged: false,
     };
-    this.onFilterMenuClose = () => {
-      this.props.onFilterMenuClose();
-    };
-    this.onFilterMenuOpen = (...args) => {
-      return this.props.onFilterMenuOpen(this.props.chartId, ...args);
-    };
-    this.onOpenDateFilterControl = (...args) => {
-      return this.onFilterMenuOpen(TIME_RANGE, ...args);
-    };
-    this.onFocus = this.onFilterMenuOpen;
-    this.onBlur = this.onFilterMenuClose;
     this.changeFilter = this.changeFilter.bind(this);
+    this.onFilterMenuOpen = this.onFilterMenuOpen.bind(this);
+    this.onOpenDateFilterControl = this.onOpenDateFilterControl.bind(this);
+    this.onFilterMenuClose = this.onFilterMenuClose.bind(this);
+  }
+
+  onFilterMenuOpen(column) {
+    return this.props.onFilterMenuOpen(this.props.chartId, column);
+  }
+
+  onOpenDateFilterControl() {
+    return this.onFilterMenuOpen(TIME_RANGE);
+  }
+
+  onFilterMenuClose() {
+    return this.props.onFilterMenuClose(this.props.chartId);
   }
 
   getControlData(controlName) {
@@ -170,8 +174,8 @@ class FilterBox extends React.Component {
               name={TIME_RANGE}
               label={label}
               description={t('Select start and end date')}
-              onChange={(...args) => {
-                this.changeFilter(TIME_RANGE, ...args);
+              onChange={newValue => {
+                this.changeFilter(TIME_RANGE, newValue);
               }}
               onOpenDateFilterControl={this.onOpenDateFilterControl}
               onCloseDateFilterControl={this.onFilterMenuClose}
@@ -282,15 +286,11 @@ class FilterBox extends React.Component {
             const style = { backgroundImage };
             return { value: opt.id, label: opt.id, style };
           })}
-        onChange={(...args) => {
-          this.changeFilter(key, ...args);
-        }}
-        onFocus={this.onFocus}
-        onBlur={this.onBlur}
-        onOpen={(...args) => {
-          this.onFilterMenuOpen(key, ...args);
-        }}
-        onClose={this.onFilterMenuClose}
+        onChange={newValue => this.changeFilter(key, newValue)}
+        onFocus={() => this.onFilterMenuOpen(key)}
+        onMenuOpen={() => this.onFilterMenuOpen(key)}
+        onBlur={this.onFilterMenuClose}
+        onMenuClose={this.onFilterMenuClose}
         selectComponent={CreatableSelect}
         noResultsText={t('No results found')}
       />