From 1d6a746a0991140d761794862a13b63df711a1d7 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Thu, 29 Apr 2021 03:49:50 -0300 Subject: [PATCH] refactor: Boostrap to AntD - Tabs (#14048) --- .../components/DashboardBuilder_spec.jsx | 37 +++++---- .../src/common/components/common.stories.tsx | 80 +------------------ .../src/components/Tabs/Tabs.stories.tsx | 68 ++++++++++++++++ .../src/components/Tabs/Tabs.tsx | 9 ++- .../DashboardBuilder/DashboardBuilder.tsx | 5 +- .../DashboardBuilder/DashboardContainer.tsx | 61 +++++++------- 6 files changed, 123 insertions(+), 137 deletions(-) create mode 100644 superset-frontend/src/components/Tabs/Tabs.stories.tsx diff --git a/superset-frontend/spec/javascripts/dashboard/components/DashboardBuilder_spec.jsx b/superset-frontend/spec/javascripts/dashboard/components/DashboardBuilder_spec.jsx index 57c0664846..4fd924e8c1 100644 --- a/superset-frontend/spec/javascripts/dashboard/components/DashboardBuilder_spec.jsx +++ b/superset-frontend/spec/javascripts/dashboard/components/DashboardBuilder_spec.jsx @@ -24,7 +24,7 @@ import fetchMock from 'fetch-mock'; import { ParentSize } from '@vx/responsive'; import { supersetTheme, ThemeProvider } from '@superset-ui/core'; import { Sticky, StickyContainer } from 'react-sticky'; -import { TabContainer, TabContent, TabPane } from 'react-bootstrap'; +import Tabs from 'src/components/Tabs'; import { DndProvider } from 'react-dnd'; import { HTML5Backend } from 'react-dnd-html5-backend'; import BuilderComponentPane from 'src/dashboard/components/BuilderComponentPane'; @@ -39,7 +39,10 @@ import { } from 'spec/fixtures/mockDashboardLayout'; import { mockStoreWithTabs, storeWithState } from 'spec/fixtures/mockStore'; import mockState from 'spec/fixtures/mockState'; -import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants'; +import { + DASHBOARD_ROOT_ID, + DASHBOARD_GRID_ID, +} from 'src/dashboard/util/constants'; fetchMock.get('glob:*/csstemplateasyncmodelview/api/read', {}); @@ -118,19 +121,17 @@ describe('DashboardBuilder', () => { }); }); - it('should render a TabContainer and TabContent', () => { + it('should render one Tabs and two TabPane', () => { const wrapper = setup({ dashboardLayout: undoableDashboardLayoutWithTabs }); const parentSize = wrapper.find(ParentSize); - expect(parentSize.find(TabContainer)).toHaveLength(1); - expect(parentSize.find(TabContent)).toHaveLength(1); + expect(parentSize.find(Tabs)).toHaveLength(1); + expect(parentSize.find(Tabs.TabPane)).toHaveLength(2); }); - it('should set animation=true, mountOnEnter=true, and unmounOnExit=false on TabContainer for perf', () => { + it('should set animated=true on Tabs for perf', () => { const wrapper = setup({ dashboardLayout: undoableDashboardLayoutWithTabs }); - const tabProps = wrapper.find(ParentSize).find(TabContainer).props(); - expect(tabProps.animation).toBe(true); - expect(tabProps.mountOnEnter).toBe(true); - expect(tabProps.unmountOnExit).toBe(false); + const tabProps = wrapper.find(ParentSize).find(Tabs).props(); + expect(tabProps.animated).toBe(true); }); it('should render a TabPane and DashboardGrid for first Tab', () => { @@ -138,10 +139,10 @@ describe('DashboardBuilder', () => { const parentSize = wrapper.find(ParentSize); const expectedCount = undoableDashboardLayoutWithTabs.present.TABS_ID.children.length; - expect(parentSize.find(TabPane)).toHaveLength(expectedCount); - expect(parentSize.find(TabPane).first().find(DashboardGrid)).toHaveLength( - 1, - ); + expect(parentSize.find(Tabs.TabPane)).toHaveLength(expectedCount); + expect( + parentSize.find(Tabs.TabPane).first().find(DashboardGrid), + ).toHaveLength(1); }); it('should render a TabPane and DashboardGrid for second Tab', () => { @@ -155,8 +156,10 @@ describe('DashboardBuilder', () => { const parentSize = wrapper.find(ParentSize); const expectedCount = undoableDashboardLayoutWithTabs.present.TABS_ID.children.length; - expect(parentSize.find(TabPane)).toHaveLength(expectedCount); - expect(parentSize.find(TabPane).at(1).find(DashboardGrid)).toHaveLength(1); + expect(parentSize.find(Tabs.TabPane)).toHaveLength(expectedCount); + expect( + parentSize.find(Tabs.TabPane).at(1).find(DashboardGrid), + ).toHaveLength(1); }); it('should render a BuilderComponentPane if editMode=false and user selects "Insert Components" pane', () => { @@ -179,7 +182,7 @@ describe('DashboardBuilder', () => { dashboardLayout: undoableDashboardLayoutWithTabs, }); - expect(wrapper.find(TabContainer).prop('activeKey')).toBe(0); + expect(wrapper.find(Tabs).prop('activeKey')).toBe(DASHBOARD_GRID_ID); wrapper .find('.dashboard-component-tabs .ant-tabs .ant-tabs-tab') diff --git a/superset-frontend/src/common/components/common.stories.tsx b/superset-frontend/src/common/components/common.stories.tsx index d706bf60a8..333d0e9747 100644 --- a/superset-frontend/src/common/components/common.stories.tsx +++ b/superset-frontend/src/common/components/common.stories.tsx @@ -18,13 +18,11 @@ */ import React, { useState, useRef, useCallback } from 'react'; import { action } from '@storybook/addon-actions'; -import { withKnobs, boolean } from '@storybook/addon-knobs'; +import { withKnobs } from '@storybook/addon-knobs'; import { CronPicker, CronError } from 'src/components/CronPicker'; import Modal from 'src/components/Modal'; import InfoTooltip from 'src/components/InfoTooltip'; -import { Dropdown } from 'src/components/Dropdown'; -import Tabs, { EditableTabs } from 'src/components/Tabs'; -import { Menu, Input, Divider } from '.'; +import { Input, Divider } from '.'; export default { title: 'Common components', @@ -45,80 +43,6 @@ export const StyledModal = () => ( ); -export const StyledTabs = () => ( - - - Tab 1 Content! - - - Tab 2 Content! - - -); - -export const StyledEditableTabs = () => ( - - - Tab 1 Content! - - - Tab 2 Content! - - -); - -export const TabsWithDropdownMenu = () => ( - - - - Item 1 - Item 2 - - } - /> - Tab with dropdown menu - - } - key="1" - disabled={boolean('Tab 1 disabled', false)} - > - Tab 1 Content! - - -); - export const StyledInfoTooltip = (args: any) => { const styles = { padding: '100px 0 0 200px', diff --git a/superset-frontend/src/components/Tabs/Tabs.stories.tsx b/superset-frontend/src/components/Tabs/Tabs.stories.tsx new file mode 100644 index 0000000000..531ffe69a5 --- /dev/null +++ b/superset-frontend/src/components/Tabs/Tabs.stories.tsx @@ -0,0 +1,68 @@ +/** + * 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. + */ +import React from 'react'; +import Tabs, { TabsProps } from '.'; + +const { TabPane } = Tabs; + +export default { + title: 'Tabs', + component: Tabs, +}; + +export const InteractiveTabs = (args: TabsProps) => ( + + + Content of Tab Pane 1 + + + Content of Tab Pane 2 + + + Content of Tab Pane 3 + + +); + +InteractiveTabs.args = { + defaultActiveKey: '1', + animated: true, + centered: false, + fullWidth: false, + allowOverflow: false, +}; + +InteractiveTabs.argTypes = { + onChange: { action: 'onChange' }, + type: { + defaultValue: 'line', + control: { + type: 'inline-radio', + options: ['line', 'card', 'editable-card'], + }, + }, +}; + +InteractiveTabs.story = { + parameters: { + knobs: { + disable: true, + }, + }, +}; diff --git a/superset-frontend/src/components/Tabs/Tabs.tsx b/superset-frontend/src/components/Tabs/Tabs.tsx index b97e4c82ab..727fe1527a 100644 --- a/superset-frontend/src/components/Tabs/Tabs.tsx +++ b/superset-frontend/src/components/Tabs/Tabs.tsx @@ -18,17 +18,17 @@ */ import React from 'react'; import { css, styled } from '@superset-ui/core'; -import { Tabs as AntdTabs } from 'src/common/components'; +import AntDTabs, { TabsProps as AntDTabsProps } from 'antd/lib/tabs'; import Icon from 'src/components/Icon'; -interface TabsProps { +export interface TabsProps extends AntDTabsProps { fullWidth?: boolean; allowOverflow?: boolean; } const notForwardedProps = ['fullWidth', 'allowOverflow']; -const StyledTabs = styled(AntdTabs, { +const StyledTabs = styled(AntDTabs, { shouldForwardProp: prop => !notForwardedProps.includes(prop), })` overflow: ${({ allowOverflow }) => (allowOverflow ? 'visible' : 'hidden')}; @@ -96,7 +96,7 @@ const StyledTabs = styled(AntdTabs, { } `; -const StyledTabPane = styled(AntdTabs.TabPane)``; +const StyledTabPane = styled(AntDTabs.TabPane)``; const Tabs = Object.assign(StyledTabs, { TabPane: StyledTabPane, @@ -104,6 +104,7 @@ const Tabs = Object.assign(StyledTabs, { Tabs.defaultProps = { fullWidth: true, + animated: true, }; const StyledEditableTabs = styled(StyledTabs)` diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index 23b332ef1f..2db91c356c 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -244,10 +244,7 @@ const DashboardBuilder: FC = () => { )} - + {editMode && } diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx index cd01209370..7d5d07d578 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx @@ -19,8 +19,8 @@ // ParentSize uses resize observer so the dashboard will update size // when its container size changes, due to e.g., builder side panel opening import { ParentSize } from '@vx/responsive'; -import { TabContainer, TabContent, TabPane } from 'react-bootstrap'; -import React, { FC, SyntheticEvent, useEffect, useState } from 'react'; +import Tabs from 'src/components/Tabs'; +import React, { FC, useEffect, useState } from 'react'; import { useSelector } from 'react-redux'; import DashboardGrid from 'src/dashboard/containers/DashboardGrid'; import getLeafComponentIdFromPath from 'src/dashboard/util/getLeafComponentIdFromPath'; @@ -33,13 +33,9 @@ import { getRootLevelTabIndex } from './utils'; type DashboardContainerProps = { topLevelTabs?: LayoutItem; - handleChangeTab: (event: SyntheticEvent) => void; }; -const DashboardContainer: FC = ({ - topLevelTabs, - handleChangeTab, -}) => { +const DashboardContainer: FC = ({ topLevelTabs }) => { const dashboardLayout = useSelector( state => state.dashboardLayout.present, ); @@ -58,6 +54,9 @@ const DashboardContainer: FC = ({ ? topLevelTabs.children : [DASHBOARD_GRID_ID]; + const min = Math.min(tabIndex, childIds.length - 1); + const activeKey = min === 0 ? DASHBOARD_GRID_ID : min.toString(); + return (
@@ -68,35 +67,29 @@ const DashboardContainer: FC = ({ the entire dashboard upon adding/removing top-level tabs, which would otherwise happen because of React's diffing algorithm */ - <>} + fullWidth={false} > - - {childIds.map((id, index) => ( - // Matching the key of the first TabPane irrespective of topLevelTabs - // lets us keep the same React component tree when !!topLevelTabs changes. - // This avoids expensive mounts/unmounts of the entire dashboard. - - - - ))} - - + {childIds.map((id, index) => ( + // Matching the key of the first TabPane irrespective of topLevelTabs + // lets us keep the same React component tree when !!topLevelTabs changes. + // This avoids expensive mounts/unmounts of the entire dashboard. + + + + ))} + )}