From 6d5f803a15d8b899acd029435c3b85acdee18dec Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 25 Sep 2020 08:18:36 +0200 Subject: [PATCH 1/5] Re-enable ESLint rule default-props-match-props-types --- superset-frontend/.eslintrc.js | 2 -- superset-frontend/src/CRUD/Field.jsx | 3 +-- .../src/SqlLab/components/ExploreCtasResultsButton.jsx | 7 +------ .../src/SqlLab/components/SqlEditorLeftBar.jsx | 2 +- .../src/SqlLab/components/TemplateParamsEditor.jsx | 2 -- superset-frontend/src/components/Hotkeys.jsx | 2 +- superset-frontend/src/components/Select/OnPasteSelect.jsx | 6 +++--- .../src/dashboard/components/ColorSchemeControlWrapper.jsx | 2 +- .../src/dashboard/components/DashboardBuilder.jsx | 2 +- .../src/dashboard/components/PropertiesModal.jsx | 2 +- superset-frontend/src/dashboard/components/SliceAdder.jsx | 2 +- superset-frontend/src/dashboard/components/SliceHeader.jsx | 1 - .../src/dashboard/components/gridComponents/Row.jsx | 5 ----- .../src/dashboard/components/gridComponents/Tabs.jsx | 1 - .../src/dashboard/components/menu/WithPopoverMenu.jsx | 1 - .../src/dashboard/containers/DashboardComponent.jsx | 7 ++++--- .../src/explore/components/QueryAndSaveBtns.jsx | 1 - .../src/explore/components/controls/ColorSchemeControl.jsx | 4 ++-- .../src/explore/components/controls/ViewportControl.jsx | 2 +- 19 files changed, 18 insertions(+), 36 deletions(-) diff --git a/superset-frontend/.eslintrc.js b/superset-frontend/.eslintrc.js index c2f918439f531..0ee38568f30df 100644 --- a/superset-frontend/.eslintrc.js +++ b/superset-frontend/.eslintrc.js @@ -122,7 +122,6 @@ module.exports = { 'padded-blocks': 0, 'prefer-arrow-callback': 0, 'prefer-destructuring': ['error', { object: true, array: false }], - 'react/default-props-match-prop-types': 0, // disabled temporarily 'react/destructuring-assignment': 0, // re-enable up for discussion 'react/forbid-prop-types': 0, 'react/jsx-filename-extension': [1, { extensions: ['.jsx', '.tsx'] }], @@ -233,7 +232,6 @@ module.exports = { 'prefer-arrow-callback': 0, 'prefer-object-spread': 1, 'prefer-destructuring': ['error', { object: true, array: false }], - 'react/default-props-match-prop-types': 0, // disabled temporarily 'react/destructuring-assignment': 0, // re-enable up for discussion 'react/forbid-prop-types': 0, 'react/jsx-filename-extension': [1, { extensions: ['.jsx', '.tsx'] }], diff --git a/superset-frontend/src/CRUD/Field.jsx b/superset-frontend/src/CRUD/Field.jsx index 3d56019fa554f..3ed439f2edeb1 100644 --- a/superset-frontend/src/CRUD/Field.jsx +++ b/superset-frontend/src/CRUD/Field.jsx @@ -39,10 +39,9 @@ const propTypes = { compact: PropTypes.bool, }; const defaultProps = { - controlProps: {}, onChange: () => {}, compact: false, - desc: null, + description: null, }; export default class Field extends React.PureComponent { diff --git a/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton.jsx b/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton.jsx index 0fac3d1159b8b..e8829caf72f6a 100644 --- a/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton.jsx +++ b/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton.jsx @@ -25,7 +25,7 @@ import { t } from '@superset-ui/core'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import Button from 'src/components/Button'; -import { exploreChart } from '../../explore/exploreUtils'; +import { exploreChart } from 'src/explore/exploreUtils'; import * as actions from '../actions/sqlLab'; const propTypes = { @@ -37,10 +37,6 @@ const propTypes = { templateParams: PropTypes.string, }; -const defaultProps = { - vizRequest: {}, -}; - class ExploreCtasResultsButton extends React.PureComponent { constructor(props) { super(props); @@ -113,7 +109,6 @@ class ExploreCtasResultsButton extends React.PureComponent { } } ExploreCtasResultsButton.propTypes = propTypes; -ExploreCtasResultsButton.defaultProps = defaultProps; function mapStateToProps({ sqlLab, common }) { return { diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar.jsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar.jsx index 552ba4ce6bb1d..66a538584b94d 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar.jsx @@ -25,7 +25,7 @@ import TableSelector from '../../components/TableSelector'; const propTypes = { queryEditor: PropTypes.object.isRequired, - height: PropTypes.number.isRequired, + height: PropTypes.number, tables: PropTypes.array, actions: PropTypes.object, database: PropTypes.object, diff --git a/superset-frontend/src/SqlLab/components/TemplateParamsEditor.jsx b/superset-frontend/src/SqlLab/components/TemplateParamsEditor.jsx index a15f5638b514d..45525768811eb 100644 --- a/superset-frontend/src/SqlLab/components/TemplateParamsEditor.jsx +++ b/superset-frontend/src/SqlLab/components/TemplateParamsEditor.jsx @@ -33,8 +33,6 @@ const propTypes = { }; const defaultProps = { - label: null, - description: null, onChange: () => {}, code: '{}', }; diff --git a/superset-frontend/src/components/Hotkeys.jsx b/superset-frontend/src/components/Hotkeys.jsx index c0d4707bf3ffd..fbe76b2e79db1 100644 --- a/superset-frontend/src/components/Hotkeys.jsx +++ b/superset-frontend/src/components/Hotkeys.jsx @@ -28,7 +28,7 @@ const propTypes = { descr: PropTypes.string.isRequired, func: PropTypes.func, }), - ).isRequired, + ), header: PropTypes.string, placement: PropTypes.string, }; diff --git a/superset-frontend/src/components/Select/OnPasteSelect.jsx b/superset-frontend/src/components/Select/OnPasteSelect.jsx index fcf1e10343ce1..e7d08aea0a79c 100644 --- a/superset-frontend/src/components/Select/OnPasteSelect.jsx +++ b/superset-frontend/src/components/Select/OnPasteSelect.jsx @@ -81,12 +81,12 @@ export default class OnPasteSelect extends React.Component { } OnPasteSelect.propTypes = { - separator: PropTypes.array.isRequired, + separator: PropTypes.array, selectWrap: PropTypes.elementType, selectRef: PropTypes.func, onChange: PropTypes.func.isRequired, - valueKey: PropTypes.string.isRequired, - labelKey: PropTypes.string.isRequired, + valueKey: PropTypes.string, + labelKey: PropTypes.string, options: PropTypes.array, isMulti: PropTypes.bool, value: PropTypes.any, diff --git a/superset-frontend/src/dashboard/components/ColorSchemeControlWrapper.jsx b/superset-frontend/src/dashboard/components/ColorSchemeControlWrapper.jsx index b2e14d1037e7a..21db6ba1a1be7 100644 --- a/superset-frontend/src/dashboard/components/ColorSchemeControlWrapper.jsx +++ b/superset-frontend/src/dashboard/components/ColorSchemeControlWrapper.jsx @@ -24,7 +24,7 @@ import { getCategoricalSchemeRegistry, t } from '@superset-ui/core'; import ColorSchemeControl from 'src/explore/components/controls/ColorSchemeControl'; const propTypes = { - onChange: PropTypes.func.isRequired, + onChange: PropTypes.func, colorScheme: PropTypes.string, }; diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder.jsx b/superset-frontend/src/dashboard/components/DashboardBuilder.jsx index ecaf3cf2ea671..91bbf3b9995e7 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder.jsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder.jsx @@ -54,7 +54,7 @@ const propTypes = { dashboardLayout: PropTypes.object.isRequired, deleteTopLevelTabs: PropTypes.func.isRequired, editMode: PropTypes.bool.isRequired, - showBuilderPane: PropTypes.func.isRequired, + showBuilderPane: PropTypes.func, colorScheme: PropTypes.string, setColorSchemeAndUnsavedChanges: PropTypes.func.isRequired, handleComponentDrop: PropTypes.func.isRequired, diff --git a/superset-frontend/src/dashboard/components/PropertiesModal.jsx b/superset-frontend/src/dashboard/components/PropertiesModal.jsx index 57f348282ee26..2637f519e219e 100644 --- a/superset-frontend/src/dashboard/components/PropertiesModal.jsx +++ b/superset-frontend/src/dashboard/components/PropertiesModal.jsx @@ -35,7 +35,7 @@ import '../stylesheets/buttons.less'; const propTypes = { dashboardId: PropTypes.number.isRequired, - show: PropTypes.bool.isRequired, + show: PropTypes.bool, onHide: PropTypes.func, colorScheme: PropTypes.object, setColorSchemeAndUnsavedChanges: PropTypes.func, diff --git a/superset-frontend/src/dashboard/components/SliceAdder.jsx b/superset-frontend/src/dashboard/components/SliceAdder.jsx index 9034a8105a380..80d80cbf400ee 100644 --- a/superset-frontend/src/dashboard/components/SliceAdder.jsx +++ b/superset-frontend/src/dashboard/components/SliceAdder.jsx @@ -39,7 +39,7 @@ const propTypes = { lastUpdated: PropTypes.number.isRequired, errorMessage: PropTypes.string, userId: PropTypes.string.isRequired, - selectedSliceIds: PropTypes.arrayOf(PropTypes.number).isRequired, + selectedSliceIds: PropTypes.arrayOf(PropTypes.number), editMode: PropTypes.bool, height: PropTypes.number, }; diff --git a/superset-frontend/src/dashboard/components/SliceHeader.jsx b/superset-frontend/src/dashboard/components/SliceHeader.jsx index 941c0ad2588f6..276d36a6dc508 100644 --- a/superset-frontend/src/dashboard/components/SliceHeader.jsx +++ b/superset-frontend/src/dashboard/components/SliceHeader.jsx @@ -53,7 +53,6 @@ const propTypes = { const defaultProps = { innerRef: null, forceRefresh: () => ({}), - removeSlice: () => ({}), updateSliceName: () => ({}), toggleExpandSlice: () => ({}), exploreChart: () => ({}), diff --git a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx index f9076bcf4ebf4..f98cac9373641 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx @@ -56,10 +56,6 @@ const propTypes = { updateComponents: PropTypes.func.isRequired, }; -const defaultProps = { - rowHeight: null, -}; - class Row extends React.PureComponent { constructor(props) { super(props); @@ -192,6 +188,5 @@ class Row extends React.PureComponent { } Row.propTypes = propTypes; -Row.defaultProps = defaultProps; export default Row; diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx index b765cbf93bcc4..1765ebb0af7dc 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx @@ -69,7 +69,6 @@ const propTypes = { }; const defaultProps = { - children: null, renderTabContent: true, renderHoverMenu: true, availableColumnCount: 0, diff --git a/superset-frontend/src/dashboard/components/menu/WithPopoverMenu.jsx b/superset-frontend/src/dashboard/components/menu/WithPopoverMenu.jsx index 42158dccfe69b..eb439011577b0 100644 --- a/superset-frontend/src/dashboard/components/menu/WithPopoverMenu.jsx +++ b/superset-frontend/src/dashboard/components/menu/WithPopoverMenu.jsx @@ -35,7 +35,6 @@ const defaultProps = { children: null, disableClick: false, onChangeFocus: null, - onPressDelete() {}, menuItems: [], isFocused: false, shouldFocus: (event, container) => diff --git a/superset-frontend/src/dashboard/containers/DashboardComponent.jsx b/superset-frontend/src/dashboard/containers/DashboardComponent.jsx index d4a403f752df9..f5399ee39b963 100644 --- a/superset-frontend/src/dashboard/containers/DashboardComponent.jsx +++ b/superset-frontend/src/dashboard/containers/DashboardComponent.jsx @@ -21,6 +21,8 @@ import PropTypes from 'prop-types'; import { bindActionCreators } from 'redux'; import { connect } from 'react-redux'; +import { logEvent } from 'src/logger/actions'; +import { addDangerToast } from 'src/messageToasts/actions'; import ComponentLookup from '../components/gridComponents'; import getDetailedComponentWidth from '../util/getDetailedComponentWidth'; import { getActiveFilters } from '../util/activeDashboardFilters'; @@ -34,8 +36,6 @@ import { handleComponentDrop, } from '../actions/dashboardLayout'; import { setDirectPathToChild, setMountedTab } from '../actions/dashboardState'; -import { logEvent } from '../../logger/actions'; -import { addDangerToast } from '../../messageToasts/actions'; const propTypes = { component: componentShape.isRequired, @@ -48,12 +48,13 @@ const propTypes = { directPathToChild: PropTypes.arrayOf(PropTypes.string), directPathLastUpdated: PropTypes.number, dashboardId: PropTypes.number.isRequired, + isComponentVisible: PropTypes.bool, }; const defaultProps = { directPathToChild: [], directPathLastUpdated: 0, - isComponentVisible: true, + // isComponentVisible: true, }; function mapStateToProps( diff --git a/superset-frontend/src/explore/components/QueryAndSaveBtns.jsx b/superset-frontend/src/explore/components/QueryAndSaveBtns.jsx index e169dc4540849..d04cf9d8c4df6 100644 --- a/superset-frontend/src/explore/components/QueryAndSaveBtns.jsx +++ b/superset-frontend/src/explore/components/QueryAndSaveBtns.jsx @@ -37,7 +37,6 @@ const propTypes = { const defaultProps = { onStop: () => {}, onSave: () => {}, - disabled: false, }; // Prolly need to move this to a global context diff --git a/superset-frontend/src/explore/components/controls/ColorSchemeControl.jsx b/superset-frontend/src/explore/components/controls/ColorSchemeControl.jsx index 4b9d6e4f451f9..0d3a56e6fd29a 100644 --- a/superset-frontend/src/explore/components/controls/ColorSchemeControl.jsx +++ b/superset-frontend/src/explore/components/controls/ColorSchemeControl.jsx @@ -35,8 +35,8 @@ const propTypes = { choices: PropTypes.oneOfType([ PropTypes.arrayOf(PropTypes.array), PropTypes.func, - ]).isRequired, - schemes: PropTypes.oneOfType([PropTypes.object, PropTypes.func]).isRequired, + ]), + schemes: PropTypes.oneOfType([PropTypes.object, PropTypes.func]), isLinear: PropTypes.bool, }; diff --git a/superset-frontend/src/explore/components/controls/ViewportControl.jsx b/superset-frontend/src/explore/components/controls/ViewportControl.jsx index 9aa6be1663bfc..11731b14c6698 100644 --- a/superset-frontend/src/explore/components/controls/ViewportControl.jsx +++ b/superset-frontend/src/explore/components/controls/ViewportControl.jsx @@ -37,7 +37,7 @@ export const DEFAULT_VIEWPORT = { const PARAMS = ['longitude', 'latitude', 'zoom', 'bearing', 'pitch']; const propTypes = { - onChange: PropTypes.func.isRequired, + onChange: PropTypes.func, value: PropTypes.shape({ longitude: PropTypes.number, latitude: PropTypes.number, From 56075a68a40c007e4a8789e38f498ca170443662 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 25 Sep 2020 08:59:04 +0200 Subject: [PATCH 2/5] Add cypress test for switching tabs --- .../integration/dashboard/tabs.test.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/tabs.test.js b/superset-frontend/cypress-base/cypress/integration/dashboard/tabs.test.js index e4dd0d0f3f9ea..903c62c7c02cd 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/tabs.test.js +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/tabs.test.js @@ -91,6 +91,25 @@ describe('Dashboard tabs', () => { }); }); + it('should switch active tab on click', () => { + cy.wait('@filterRequest'); + cy.wait('@treemapRequest'); + + cy.get('.dashboard-component-tabs') + .first() + .find('ul.nav.nav-tabs li') + .first() + .as('firstTab') + .last() + .as('lastTab'); + + cy.get('@firstTab').click().should('have.class', 'active'); + cy.get('@secondTab').should('not.have.class', 'active'); + + cy.get('@secondTab').click().should('have.class', 'active'); + cy.get('@firstTab').should('not.have.class', 'active'); + }); + it('should load charts when tab is visible', () => { // landing in first tab, should see 2 charts cy.wait('@filterRequest'); From ce150679921fe6f2bc7945415b3016f372caeecf Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 25 Sep 2020 09:02:15 +0200 Subject: [PATCH 3/5] fix --- .../src/dashboard/containers/DashboardComponent.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/containers/DashboardComponent.jsx b/superset-frontend/src/dashboard/containers/DashboardComponent.jsx index f5399ee39b963..b7ac2942e09ab 100644 --- a/superset-frontend/src/dashboard/containers/DashboardComponent.jsx +++ b/superset-frontend/src/dashboard/containers/DashboardComponent.jsx @@ -54,7 +54,7 @@ const propTypes = { const defaultProps = { directPathToChild: [], directPathLastUpdated: 0, - // isComponentVisible: true, + isComponentVisible: true, }; function mapStateToProps( From 7a6155fdaaaeef5d5e5e92f8a5696405a0f1ad7a Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 25 Sep 2020 09:38:30 +0200 Subject: [PATCH 4/5] Typo fix --- .../cypress-base/cypress/integration/dashboard/tabs.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/tabs.test.js b/superset-frontend/cypress-base/cypress/integration/dashboard/tabs.test.js index 903c62c7c02cd..6c82ce2b68c97 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/tabs.test.js +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/tabs.test.js @@ -104,9 +104,9 @@ describe('Dashboard tabs', () => { .as('lastTab'); cy.get('@firstTab').click().should('have.class', 'active'); - cy.get('@secondTab').should('not.have.class', 'active'); + cy.get('@lastTab').should('not.have.class', 'active'); - cy.get('@secondTab').click().should('have.class', 'active'); + cy.get('@lastTab').click().should('have.class', 'active'); cy.get('@firstTab').should('not.have.class', 'active'); }); From 0831aaff84ae8d64d6fb24ae33fc27110dda99e8 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 25 Sep 2020 11:14:37 +0200 Subject: [PATCH 5/5] Test fix --- .../cypress/integration/dashboard/tabs.test.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/tabs.test.js b/superset-frontend/cypress-base/cypress/integration/dashboard/tabs.test.js index 6c82ce2b68c97..94b0bb0ccd692 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/tabs.test.js +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/tabs.test.js @@ -98,16 +98,13 @@ describe('Dashboard tabs', () => { cy.get('.dashboard-component-tabs') .first() .find('ul.nav.nav-tabs li') - .first() - .as('firstTab') - .last() - .as('lastTab'); + .as('tabs'); - cy.get('@firstTab').click().should('have.class', 'active'); - cy.get('@lastTab').should('not.have.class', 'active'); + cy.get('@tabs').first().click().should('have.class', 'active'); + cy.get('@tabs').last().should('not.have.class', 'active'); - cy.get('@lastTab').click().should('have.class', 'active'); - cy.get('@firstTab').should('not.have.class', 'active'); + cy.get('@tabs').last().click().should('have.class', 'active'); + cy.get('@tabs').first().should('not.have.class', 'active'); }); it('should load charts when tab is visible', () => {