From ee3c639a151e59d0b56bacce7662d00999afd57f Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Sat, 31 Oct 2020 06:05:31 +0100 Subject: [PATCH] refactor: Replace react-bootstrap tabs with Antd tabs (#11118) * Replace tabs in BuilderComponentPane * Replace tabs in ControlPanelsContainer * Replace tabs in AdhocMetricEditPopover * Replace Tabs in DatasourceEditor * Replace tabs in AdhocFilterEditPopover * Replace tabs in DateFilterControl * Bug fix * Change Tab styles * Fix tests * Fix cypress tests * Lint fix * Fix tests * Change Tabs style in ControlPanelsContainer * Change tabs content height * Lint fix * Add data test * Fix e2e test * Move Tabs file to separate dir * Fix after rebase * Fix e2e tests * Fix after rebase --- .../integration/dashboard/edit_mode.test.js | 5 +- .../integration/explore/AdhocFilters.test.ts | 6 + .../integration/explore/AdhocMetrics.test.ts | 8 - .../integration/explore/control.test.ts | 6 +- .../datasource/DatasourceEditor_spec.jsx | 2 +- .../AdhocFilterEditPopover_spec.jsx | 6 +- .../components/DateFilterControl_spec.jsx | 11 +- .../src/common/components/{ => Tabs}/Tabs.tsx | 16 +- .../src/common/components/Tabs/index.ts | 20 +++ .../components/BuilderComponentPane.jsx | 13 +- .../src/datasource/DatasourceEditor.jsx | 140 +++++++++--------- .../components/AdhocFilterEditPopover.jsx | 20 +-- .../components/AdhocMetricEditPopover.jsx | 24 +-- .../components/ControlPanelsContainer.jsx | 34 +++-- .../components/controls/DateFilterControl.jsx | 17 ++- 15 files changed, 188 insertions(+), 140 deletions(-) rename superset-frontend/src/common/components/{ => Tabs}/Tabs.tsx (91%) create mode 100644 superset-frontend/src/common/components/Tabs/index.ts diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js b/superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js index c628837ebbf59..4832c445d98c5 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js @@ -47,8 +47,9 @@ describe('Dashboard edit mode', () => { }); cy.get('[data-test="dashboard-builder-component-pane-tabs-navigation"]') - .children() - .last() + .within(() => { + cy.get('.ant-tabs-tab').last(); + }) .click(); // find box plot is available from list diff --git a/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts index 9b7c5ae61c473..d9057d3823a0a 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts @@ -43,6 +43,12 @@ describe('AdhocFilters', () => { cy.get('input[type=text]').focus().type('name{enter}'); }); + // antd tabs do lazy loading, so we need to click on tab with ace editor + cy.get('#filter-edit-popover').within(() => { + cy.get('.ant-tabs-tab').contains('Custom SQL').click(); + cy.get('.ant-tabs-tab').contains('Simple').click(); + }); + cy.get('script').then(nodes => { // should load new script chunks for SQL editor expect(nodes.length).to.greaterThan(numScripts); diff --git a/superset-frontend/cypress-base/cypress/integration/explore/AdhocMetrics.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/AdhocMetrics.test.ts index e839fc26c32df..7c085c33190c3 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/AdhocMetrics.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/AdhocMetrics.test.ts @@ -72,11 +72,6 @@ describe('AdhocMetrics', () => { .should('have.text', 'num') .click(); - cy.get('[data-test=option-label]') - .should('have.text', 'SUM(num)') - .first() - .click(); - // add custom SQL cy.get('#adhoc-metric-edit-tabs-tab-SQL').click(); cy.get('[data-test=metrics-edit-popover]').within(() => { @@ -103,9 +98,6 @@ describe('AdhocMetrics', () => { cy.get('[data-test=metrics]') .find('[data-test="metric-option"]') .should('have.length', 2); - cy.get('[data-test=metrics]').within(() => { - cy.contains('[data-test="metric-option"]', 'SUM(sum_girls)').click(); - }); cy.get('#metrics-edit-popover').within(() => { cy.get('#adhoc-metric-edit-tabs-tab-SQL').click(); diff --git a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts index 4cf22dffa2d36..e4bc157f001e1 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts @@ -70,7 +70,9 @@ describe('Datasource control', () => { cy.get('[data-test="datasource-menu-trigger"]').click(); cy.get('[data-test="edit-dataset"]').click(); cy.get('.ant-modal-content').within(() => { - cy.get('a[role="tab"]').contains('Metrics').click(); + cy.get('[data-test="collection-tab-Metrics"]') + .contains('Metrics') + .click(); }); cy.get(`input[value="${newMetricName}"]`) .closest('tr') @@ -140,7 +142,7 @@ describe('Time range filter', () => { }); cy.get('#filter-popover').within(() => { - cy.get('div.tab-pane.active').within(() => { + cy.get('div.ant-tabs-tabpane-active').within(() => { cy.get('div.PopoverSection :not(.dimmed)').within(() => { cy.get('input[value="100 years ago"]'); cy.get('input[value="now"]'); diff --git a/superset-frontend/spec/javascripts/datasource/DatasourceEditor_spec.jsx b/superset-frontend/spec/javascripts/datasource/DatasourceEditor_spec.jsx index ce0d3ea34f29c..7fb2b2a01ce61 100644 --- a/superset-frontend/spec/javascripts/datasource/DatasourceEditor_spec.jsx +++ b/superset-frontend/spec/javascripts/datasource/DatasourceEditor_spec.jsx @@ -17,12 +17,12 @@ * under the License. */ import React from 'react'; -import { Tabs } from 'react-bootstrap'; import { shallow } from 'enzyme'; import configureStore from 'redux-mock-store'; import fetchMock from 'fetch-mock'; import thunk from 'redux-thunk'; +import Tabs from 'src/common/components/Tabs'; import DatasourceEditor from 'src/datasource/DatasourceEditor'; import Field from 'src/CRUD/Field'; import mockDatasource from '../../fixtures/mockDatasource'; diff --git a/superset-frontend/spec/javascripts/explore/components/AdhocFilterEditPopover_spec.jsx b/superset-frontend/spec/javascripts/explore/components/AdhocFilterEditPopover_spec.jsx index c8e91c6f7446f..c28ac9a425670 100644 --- a/superset-frontend/spec/javascripts/explore/components/AdhocFilterEditPopover_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/AdhocFilterEditPopover_spec.jsx @@ -20,9 +20,9 @@ import React from 'react'; import sinon from 'sinon'; import { shallow } from 'enzyme'; -import { Tab, Tabs } from 'react-bootstrap'; import Button from 'src/components/Button'; +import Tabs from 'src/common/components/Tabs'; import AdhocFilter, { EXPRESSION_TYPES, CLAUSES, @@ -82,7 +82,7 @@ describe('AdhocFilterEditPopover', () => { it('renders simple tab content by default', () => { const { wrapper } = setup(); expect(wrapper.find(Tabs)).toExist(); - expect(wrapper.find(Tab)).toHaveLength(2); + expect(wrapper.find(Tabs.TabPane)).toHaveLength(2); expect(wrapper.find(Button)).toHaveLength(2); expect(wrapper.find(AdhocFilterEditPopoverSimpleTabContent)).toHaveLength( 1, @@ -92,7 +92,7 @@ describe('AdhocFilterEditPopover', () => { it('renders sql tab content when the adhoc filter expressionType is sql', () => { const { wrapper } = setup({ adhocFilter: sqlAdhocFilter }); expect(wrapper.find(Tabs)).toExist(); - expect(wrapper.find(Tab)).toHaveLength(2); + expect(wrapper.find(Tabs.TabPane)).toHaveLength(2); expect(wrapper.find(Button)).toHaveLength(2); expect(wrapper.find(AdhocFilterEditPopoverSqlTabContent)).toExist(); }); diff --git a/superset-frontend/spec/javascripts/explore/components/DateFilterControl_spec.jsx b/superset-frontend/spec/javascripts/explore/components/DateFilterControl_spec.jsx index fd1984d6a878a..49ac9506833c4 100644 --- a/superset-frontend/spec/javascripts/explore/components/DateFilterControl_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/DateFilterControl_spec.jsx @@ -18,11 +18,12 @@ */ /* eslint-disable no-unused-expressions */ import React from 'react'; -import { OverlayTrigger, Tab, Tabs, Radio } from 'react-bootstrap'; +import { OverlayTrigger, Radio } from 'react-bootstrap'; import sinon from 'sinon'; import { styledMount as mount } from 'spec/helpers/theming'; import Popover from 'src/common/components/Popover'; +import Tabs from 'src/common/components/Tabs'; import Label from 'src/components/Label'; import DateFilterControl from 'src/explore/components/controls/DateFilterControl'; import ControlHeader from 'src/explore/components/ControlHeader'; @@ -85,13 +86,13 @@ describe('DateFilterControl', () => { const popoverContentWrapper = mount(popoverContent); expect(popoverContentWrapper.find(Tabs)).toExist(); - expect(popoverContentWrapper.find(Tab)).toHaveLength(2); + expect(popoverContentWrapper.find(Tabs.TabPane)).toHaveLength(2); }); it('renders default time options', () => { const popoverContent = wrapper.find(Popover).first().props().content; const popoverContentWrapper = mount(popoverContent); - const defaultTab = popoverContentWrapper.find(Tab).first(); + const defaultTab = popoverContentWrapper.find(Tabs.TabPane).first(); expect(defaultTab.find(Radio)).toExist(); expect(defaultTab.find(Radio)).toHaveLength(6); @@ -100,7 +101,7 @@ describe('DateFilterControl', () => { it('renders tooltips over timeframe options', () => { const popoverContent = wrapper.find(Popover).first().props().content; const popoverContentWrapper = mount(popoverContent); - const defaultTab = popoverContentWrapper.find(Tab).first(); + const defaultTab = popoverContentWrapper.find(Tabs.TabPane).first(); const radioTrigger = defaultTab.find(OverlayTrigger); expect(radioTrigger).toExist(); @@ -110,7 +111,7 @@ describe('DateFilterControl', () => { it('renders the correct time range in tooltip', () => { const popoverContent = wrapper.find(Popover).first().props().content; const popoverContentWrapper = mount(popoverContent); - const defaultTab = popoverContentWrapper.find(Tab).first(); + const defaultTab = popoverContentWrapper.find(Tabs.TabPane).first(); const triggers = defaultTab.find(OverlayTrigger); const expectedLabels = { diff --git a/superset-frontend/src/common/components/Tabs.tsx b/superset-frontend/src/common/components/Tabs/Tabs.tsx similarity index 91% rename from superset-frontend/src/common/components/Tabs.tsx rename to superset-frontend/src/common/components/Tabs/Tabs.tsx index 7627354651fcb..027f286665a06 100644 --- a/superset-frontend/src/common/components/Tabs.tsx +++ b/superset-frontend/src/common/components/Tabs/Tabs.tsx @@ -30,6 +30,10 @@ const notForwardedProps = ['fullWidth']; const StyledTabs = styled(AntdTabs, { shouldForwardProp: prop => !notForwardedProps.includes(prop), })` + .ant-tabs-content-holder { + overflow: auto; + } + .ant-tabs-tab { flex: 1 1 auto; @@ -120,5 +124,15 @@ EditableTabs.TabPane.defaultProps = { ), }; +const StyledCardTabs = styled(EditableTabs)``; + +const CardTabs = Object.assign(StyledCardTabs, { + TabPane: StyledTabPane, +}); + +CardTabs.defaultProps = { + type: 'card', +}; + export default Tabs; -export { EditableTabs }; +export { CardTabs, EditableTabs }; diff --git a/superset-frontend/src/common/components/Tabs/index.ts b/superset-frontend/src/common/components/Tabs/index.ts new file mode 100644 index 0000000000000..32bc0c3dd28ea --- /dev/null +++ b/superset-frontend/src/common/components/Tabs/index.ts @@ -0,0 +1,20 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +export * from './Tabs'; +export { default } from './Tabs'; diff --git a/superset-frontend/src/dashboard/components/BuilderComponentPane.jsx b/superset-frontend/src/dashboard/components/BuilderComponentPane.jsx index df18290b84d89..a693f8726853d 100644 --- a/superset-frontend/src/dashboard/components/BuilderComponentPane.jsx +++ b/superset-frontend/src/dashboard/components/BuilderComponentPane.jsx @@ -19,7 +19,7 @@ /* eslint-env browser */ import PropTypes from 'prop-types'; import React from 'react'; -import { Tabs, Tab } from 'react-bootstrap'; +import Tabs from 'src/common/components/Tabs'; import { StickyContainer, Sticky } from 'react-sticky'; import { ParentSize } from '@vx/responsive'; @@ -48,23 +48,24 @@ class BuilderComponentPane extends React.PureComponent { const { isSticky } = this.props; return ( - + - - + + - + ); } diff --git a/superset-frontend/src/datasource/DatasourceEditor.jsx b/superset-frontend/src/datasource/DatasourceEditor.jsx index 30fb17a2c4959..553066203a69d 100644 --- a/superset-frontend/src/datasource/DatasourceEditor.jsx +++ b/superset-frontend/src/datasource/DatasourceEditor.jsx @@ -18,10 +18,11 @@ */ import React from 'react'; import PropTypes from 'prop-types'; -import { Alert, Badge, Col, Radio, Tabs, Tab, Well } from 'react-bootstrap'; +import { Alert, Badge, Col, Radio, Well } from 'react-bootstrap'; import shortid from 'shortid'; import { styled, SupersetClient, t } from '@superset-ui/core'; +import Tabs from 'src/common/components/Tabs'; import Button from 'src/components/Button'; import CertifiedIconWithTooltip from 'src/components/CertifiedIconWithTooltip'; import DatabaseSelector from 'src/components/DatabaseSelector'; @@ -596,11 +597,9 @@ class DatasourceEditor extends React.PureComponent { const { datasource } = this.state; const { spatials, all_cols: allCols } = datasource; return ( - - } - eventKey={4} + } + key={4} > - + ); } @@ -905,94 +904,89 @@ class DatasourceEditor extends React.PureComponent { - - {activeTabKey === 0 && this.renderSourceFieldset()} - - + {this.renderSourceFieldset()} + + } - eventKey={1} + key={1} > - {activeTabKey === 1 && this.renderMetricCollection()} - - + } - eventKey={2} + key={2} > - {activeTabKey === 2 && ( -
- - this.setColumns({ databaseColumns }) - } - /> - - {this.state.metadataLoading && } -
- )} - - + + this.setColumns({ databaseColumns }) + } + /> + + {this.state.metadataLoading && } + + + } - eventKey={3} + key={3} > - {activeTabKey === 3 && ( - - this.setColumns({ calculatedColumns }) - } - editableColumnName - showExpression - allowAddItem - allowEditDataType - itemGenerator={() => ({ - column_name: '', - filterable: true, - groupby: true, - expression: '', - __expanded: true, - })} - /> - )} - - - {activeTabKey === 4 && ( -
- - {this.renderSettingsFieldset()} - - - {this.renderAdvancedFieldset()} - -
- )} - + + this.setColumns({ calculatedColumns }) + } + editableColumnName + showExpression + allowAddItem + allowEditDataType + itemGenerator={() => ({ + column_name: '', + filterable: true, + groupby: true, + expression: '', + __expanded: true, + })} + /> + + +
+ + {this.renderSettingsFieldset()} + + + {this.renderAdvancedFieldset()} + +
+ ); diff --git a/superset-frontend/src/explore/components/AdhocFilterEditPopover.jsx b/superset-frontend/src/explore/components/AdhocFilterEditPopover.jsx index 34f885cc6c8a9..11b5b6924a4fa 100644 --- a/superset-frontend/src/explore/components/AdhocFilterEditPopover.jsx +++ b/superset-frontend/src/explore/components/AdhocFilterEditPopover.jsx @@ -18,10 +18,10 @@ */ import React from 'react'; import PropTypes from 'prop-types'; -import { Tab, Tabs } from 'react-bootstrap'; import Button from 'src/components/Button'; import { t } from '@superset-ui/core'; +import Tabs from 'src/common/components/Tabs'; import columnType from '../propTypes/columnType'; import adhocMetricType from '../propTypes/adhocMetricType'; import AdhocFilter, { EXPRESSION_TYPES } from '../AdhocFilter'; @@ -46,7 +46,7 @@ const propTypes = { }; const startingWidth = 300; -const startingHeight = 190; +const startingHeight = 240; export default class AdhocFilterEditPopover extends React.Component { constructor(props) { @@ -144,10 +144,10 @@ export default class AdhocFilterEditPopover extends React.Component { data-test="adhoc-filter-edit-tabs" style={{ height: this.state.height, width: this.state.width }} > - - - + {!this.props.datasource || this.props.datasource.type !== 'druid' ? ( @@ -176,7 +176,7 @@ export default class AdhocFilterEditPopover extends React.Component { Custom SQL Filters are not available on druid datasources )} - +
)} - +