refactor: Replace react-bootstrap tabs with Antd tabs (apache#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
auxten · Nov 20, 2020 · ee3c639 · ee3c639
1 parent efb97f5
commit ee3c639
Show file tree

Hide file tree

Showing 15 changed files with 188 additions and 140 deletions.
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/...t-frontend/src/common/components/Tabs.tsx → ...ntend/src/common/components/Tabs/Tabs.tsx b/...t-frontend/src/common/components/Tabs.tsx → ...ntend/src/common/components/Tabs/Tabs.tsx
@@ -30,6 +30,10 @@ const notForwardedProps = ['fullWidth'];
 const StyledTabs = styled(AntdTabs, {
   shouldForwardProp: prop => !notForwardedProps.includes(prop),
 })<TabsProps>`
+  .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
@@ -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
@@ -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 (
       <Tabs
-        className="m-t-10 tabs-components"
         id="tabs"
+        className="tabs-components"
+        style={{ marginTop: '10px' }}
         data-test="dashboard-builder-component-pane-tabs-navigation"
       >
-        <Tab eventKey={1} title={t('Components')}>
+        <Tabs.TabPane key={1} tab={t('Components')}>
           <NewTabs />
           <NewRow />
           <NewColumn />
           <NewHeader />
           <NewMarkdown />
           <NewDivider />
-        </Tab>
-        <Tab eventKey={2} title={t('Charts')} className="tab-charts">
+        </Tabs.TabPane>
+        <Tabs.TabPane key={2} tab={t('Charts')} className="tab-charts">
           <SliceAdder
             height={height + (isSticky ? SUPERSET_HEADER_HEIGHT : 0)}
           />
-        </Tab>
+        </Tabs.TabPane>
       </Tabs>
     );
   }