From 1b2611c211e42b077e18c7b672e33cfb77681089 Mon Sep 17 00:00:00 2001 From: Geido <60598000+geido@users.noreply.github.com> Date: Tue, 26 Jan 2021 00:05:19 +0100 Subject: [PATCH] refactor(explore): Enhance Dataset and Control panel Collapse components (#12218) --- .../integration/explore/advanced.test.ts | 4 +- .../integration/explore/control.test.ts | 2 +- .../components/ControlPanelSection_spec.jsx | 69 ----------- .../ControlPanelsContainer_spec.jsx | 4 +- .../src/common/components/Collapse.tsx | 36 +++++- .../src/common/components/common.stories.tsx | 10 ++ .../components/ControlPanelSection.jsx | 117 ------------------ .../components/ControlPanelsContainer.jsx | 64 ++++++++-- .../explore/components/DatasourcePanel.tsx | 40 +----- superset-frontend/src/explore/main.less | 1 - 10 files changed, 107 insertions(+), 240 deletions(-) delete mode 100644 superset-frontend/spec/javascripts/explore/components/ControlPanelSection_spec.jsx delete mode 100644 superset-frontend/src/explore/components/ControlPanelSection.jsx diff --git a/superset-frontend/cypress-base/cypress/integration/explore/advanced.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/advanced.test.ts index c27f054c2a8e1..7a50b6e934a6e 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/advanced.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/advanced.test.ts @@ -28,7 +28,7 @@ describe('Advanced analytics', () => { cy.visitChartByName('Num Births Trend'); cy.verifySliceSuccess({ waitAlias: '@postJson' }); - cy.get('.panel-title').contains('Advanced Analytics').click(); + cy.get('.ant-collapse-header').contains('Advanced Analytics').click(); cy.get('[data-test=time_compare]').find('.Select__control').click(); cy.get('[data-test=time_compare]') @@ -47,7 +47,7 @@ describe('Advanced analytics', () => { chartSelector: 'svg', }); - cy.get('.panel-title').contains('Advanced Analytics').click(); + cy.get('.ant-collapse-header').contains('Advanced Analytics').click(); cy.get('[data-test=time_compare]') .find('.Select__multi-value__label') .contains('28 days'); 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 8c9d3e7b3f315..6ab462f32b63c 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts @@ -112,7 +112,7 @@ describe('VizType control', () => { // should load mathjs for line chart cy.get('script[src*="mathjs"]').should('have.length', 1); cy.get('script').then(nodes => { - expect(nodes.length).to.greaterThan(numScripts); + expect(nodes.length).to.eq(numScripts); }); cy.get('button[data-test="run-query-button"]').click(); diff --git a/superset-frontend/spec/javascripts/explore/components/ControlPanelSection_spec.jsx b/superset-frontend/spec/javascripts/explore/components/ControlPanelSection_spec.jsx deleted file mode 100644 index 521a5532efaa2..0000000000000 --- a/superset-frontend/spec/javascripts/explore/components/ControlPanelSection_spec.jsx +++ /dev/null @@ -1,69 +0,0 @@ -/** - * 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 { shallow } from 'enzyme'; -import { Panel } from 'react-bootstrap'; -import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; -import ControlPanelSection from 'src/explore/components/ControlPanelSection'; - -const defaultProps = { - children:
a child element
, -}; - -const optionalProps = { - label: 'my label', - description: 'my description', - tooltip: 'my tooltip', -}; - -describe('ControlPanelSection', () => { - let wrapper; - let props; - it('is a valid element', () => { - expect( - React.isValidElement(), - ).toBe(true); - }); - - it('renders a Panel component', () => { - wrapper = shallow(); - expect(wrapper.find(Panel)).toExist(); - }); - - describe('with optional props', () => { - beforeEach(() => { - props = Object.assign(defaultProps, optionalProps); - wrapper = shallow(); - }); - - it('renders a label if present', () => { - expect( - wrapper - .find('[data-test="clickable-control-panel-section-title"]') - .text(), - ).toContain('my label'); - }); - - it('renders a InfoTooltipWithTrigger if label and tooltip is present', () => { - expect( - wrapper.find(Panel).dive().find(InfoTooltipWithTrigger), - ).toHaveLength(1); - }); - }); -}); diff --git a/superset-frontend/spec/javascripts/explore/components/ControlPanelsContainer_spec.jsx b/superset-frontend/spec/javascripts/explore/components/ControlPanelsContainer_spec.jsx index 7063e64fdd3eb..35de5b6c5b349 100644 --- a/superset-frontend/spec/javascripts/explore/components/ControlPanelsContainer_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/ControlPanelsContainer_spec.jsx @@ -22,7 +22,7 @@ import { getChartControlPanelRegistry, t } from '@superset-ui/core'; import { defaultControls } from 'src/explore/store'; import { getFormDataFromControls } from 'src/explore/controlUtils'; import { ControlPanelsContainer } from 'src/explore/components/ControlPanelsContainer'; -import ControlPanelSection from 'src/explore/components/ControlPanelSection'; +import Collapse from 'src/common/components/Collapse'; describe('ControlPanelsContainer', () => { let wrapper; @@ -91,6 +91,6 @@ describe('ControlPanelsContainer', () => { it('renders ControlPanelSections', () => { wrapper = shallow(); - expect(wrapper.find(ControlPanelSection)).toHaveLength(5); + expect(wrapper.find(Collapse.Panel)).toHaveLength(5); }); }); diff --git a/superset-frontend/src/common/components/Collapse.tsx b/superset-frontend/src/common/components/Collapse.tsx index b901bf4d5ce71..c6daa77cd1755 100644 --- a/superset-frontend/src/common/components/Collapse.tsx +++ b/superset-frontend/src/common/components/Collapse.tsx @@ -26,11 +26,12 @@ interface CollapseProps extends AntdCollapseProps { light?: boolean; bigger?: boolean; bold?: boolean; + animateArrows?: boolean; } const Collapse = Object.assign( // eslint-disable-next-line @typescript-eslint/no-unused-vars - styled(({ light, bigger, bold, ...props }: CollapseProps) => ( + styled(({ light, bigger, bold, animateArrows, ...props }: CollapseProps) => ( ))` height: 100%; @@ -48,6 +49,20 @@ const Collapse = Object.assign( font-size: ${({ bigger, theme }) => bigger ? `${theme.gridUnit * 4}px` : 'inherit'}; + .ant-collapse-arrow svg { + transition: ${({ animateArrows }) => + animateArrows ? 'transform 0.24s' : 'none'}; + } + + ${({ expandIconPosition }) => + expandIconPosition && + expandIconPosition === 'right' && + ` + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(90deg) !important; + } + `} + ${({ light, theme }) => light && ` @@ -56,6 +71,13 @@ const Collapse = Object.assign( color: ${theme.colors.grayscale.light4}; } `} + + ${({ ghost, bordered, theme }) => + ghost && + bordered && + ` + border-bottom: 1px solid ${theme.colors.grayscale.light3}; + `} } .ant-collapse-content { height: 100%; @@ -68,6 +90,18 @@ const Collapse = Object.assign( } } } + .ant-collapse-item-active { + .ant-collapse-header { + ${({ expandIconPosition }) => + expandIconPosition && + expandIconPosition === 'right' && + ` + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(-90deg) !important; + } + `} + } + } `, { Panel: AntdCollapse.Panel, diff --git a/superset-frontend/src/common/components/common.stories.tsx b/superset-frontend/src/common/components/common.stories.tsx index 6c9abb0fbf736..f46ded72c6f25 100644 --- a/superset-frontend/src/common/components/common.stories.tsx +++ b/superset-frontend/src/common/components/common.stories.tsx @@ -322,6 +322,16 @@ export const CollapseTextLight = () => ( ); +export const CollapseAnimateArrows = () => ( + + + Hi! I am a sample content + + + Hi! I am another sample content + + +); export function StyledCronPicker() { // @ts-ignore const inputRef = useRef(null); diff --git a/superset-frontend/src/explore/components/ControlPanelSection.jsx b/superset-frontend/src/explore/components/ControlPanelSection.jsx deleted file mode 100644 index 43af950745ae7..0000000000000 --- a/superset-frontend/src/explore/components/ControlPanelSection.jsx +++ /dev/null @@ -1,117 +0,0 @@ -/** - * 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 PropTypes from 'prop-types'; -import { Panel } from 'react-bootstrap'; -import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; -import { styled } from '@superset-ui/core'; - -const propTypes = { - label: PropTypes.string, - description: PropTypes.string, - children: PropTypes.node.isRequired, - startExpanded: PropTypes.bool, - hasErrors: PropTypes.bool, -}; - -const defaultProps = { - label: null, - description: null, - startExpanded: false, - hasErrors: false, -}; - -const StyledPanelTitle = styled(Panel.Title)` - & > div { - display: flex; - align-items: center; - justify-content: space-between; - } -`; - -export default class ControlPanelSection extends React.Component { - constructor(props) { - super(props); - this.state = { expanded: this.props.startExpanded }; - this.toggleExpand = this.toggleExpand.bind(this); - } - - toggleExpand() { - this.setState(prevState => ({ expanded: !prevState.expanded })); - } - - renderHeader() { - const { label, description, hasErrors } = this.props; - return ( - label && ( -
- - - {label} - {' '} - {description && ( - - )} - {hasErrors && ( - - )} - - -
- ) - ); - } - - render() { - return ( - - - {this.renderHeader()} - - - {this.props.children} - - - ); - } -} - -ControlPanelSection.propTypes = propTypes; -ControlPanelSection.defaultProps = defaultProps; diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.jsx b/superset-frontend/src/explore/components/ControlPanelsContainer.jsx index a534438b0aeed..e7b036edceb73 100644 --- a/superset-frontend/src/explore/components/ControlPanelsContainer.jsx +++ b/superset-frontend/src/explore/components/ControlPanelsContainer.jsx @@ -25,9 +25,10 @@ import { Alert } from 'react-bootstrap'; import { t, styled, getChartControlPanelRegistry } from '@superset-ui/core'; import Tabs from 'src/common/components/Tabs'; +import { Collapse } from 'src/common/components'; import { PluginContext } from 'src/components/DynamicPlugins'; import Loading from 'src/components/Loading'; -import ControlPanelSection from './ControlPanelSection'; +import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import ControlRow from './ControlRow'; import Control from './Control'; import { sectionsToRender } from '../controlUtils'; @@ -75,8 +76,10 @@ const ControlPanelsTabs = styled(Tabs)` .ant-tabs-content-holder { overflow: visible; } + .ant-tabs-tabpane { + height: 100%; + } `; - class ControlPanelsContainer extends React.Component { // trigger updates to the component when async plugins load static contextType = PluginContext; @@ -96,6 +99,13 @@ class ControlPanelsContainer extends React.Component { ); } + sectionsToExpand(sections) { + return sections.reduce( + (acc, cur) => (cur.expanded ? [...acc, cur.label] : acc), + [], + ); + } + removeAlert() { this.props.actions.removeControlPanelAlert(); } @@ -137,6 +147,7 @@ class ControlPanelsContainer extends React.Component { renderControlPanelSection(section) { const { controls } = this.props; + const { label, description } = section; const hasErrors = section.controlSetRows.some(rows => rows.some( @@ -146,14 +157,27 @@ class ControlPanelsContainer extends React.Component { controls[s].validationErrors.length > 0, ), ); + const PanelHeader = () => ( + + {label}{' '} + {description && ( + + )} + {hasErrors && ( + + )} + + ); return ( - {section.controlSetRows.map((controlSets, i) => { const renderedControls = controlSets @@ -188,7 +212,7 @@ class ControlPanelsContainer extends React.Component { /> ); })} - + ); } @@ -223,7 +247,13 @@ class ControlPanelsContainer extends React.Component { displaySectionsToRender.push(section); } }); + const showCustomizeTab = displaySectionsToRender.length > 0; + const expandedQuerySections = this.sectionsToExpand(querySectionsToRender); + const expandedCustomSections = this.sectionsToExpand( + displaySectionsToRender, + ); + return ( {this.props.alert && ( @@ -245,11 +275,25 @@ class ControlPanelsContainer extends React.Component { fullWidth={showCustomizeTab} > - {querySectionsToRender.map(this.renderControlPanelSection)} + + {querySectionsToRender.map(this.renderControlPanelSection)} + {showCustomizeTab && ( - {displaySectionsToRender.map(this.renderControlPanelSection)} + + {displaySectionsToRender.map(this.renderControlPanelSection)} + )} diff --git a/superset-frontend/src/explore/components/DatasourcePanel.tsx b/superset-frontend/src/explore/components/DatasourcePanel.tsx index dd81ce09d7767..c35bd0231ce34 100644 --- a/superset-frontend/src/explore/components/DatasourcePanel.tsx +++ b/superset-frontend/src/explore/components/DatasourcePanel.tsx @@ -83,44 +83,9 @@ const DatasourceContainer = styled.div` max-height: 100%; .ant-collapse { height: auto; - border-bottom: 1px solid ${({ theme }) => theme.colors.grayscale.light2}; - padding-bottom: ${({ theme }) => theme.gridUnit * 2}px; - background-color: ${({ theme }) => theme.colors.grayscale.light4}; - } - .ant-collapse > .ant-collapse-item > .ant-collapse-header { - padding-left: ${({ theme }) => theme.gridUnit * 2}px; - padding-bottom: 0px; - } - .ant-collapse-item { - background-color: ${({ theme }) => theme.colors.grayscale.light4}; - .anticon.anticon-right.ant-collapse-arrow > svg { - transform: rotate(90deg) !important; - margin-right: ${({ theme }) => theme.gridUnit * -2}px; - } - } - .ant-collapse-item.ant-collapse-item-active { - .anticon.anticon-right.ant-collapse-arrow > svg { - transform: rotate(-90deg) !important; - } - .ant-collapse-header { - border: 0; - } - } - .header { - font-size: ${({ theme }) => theme.typography.sizes.l}px; - margin-left: ${({ theme }) => theme.gridUnit * -2}px; - } - .ant-collapse-borderless - > .ant-collapse-item - > .ant-collapse-content - > .ant-collapse-content-box { - padding: 0px; } .field-selections { - padding: ${({ theme }) => - `${2 * theme.gridUnit}px ${2 * theme.gridUnit}px ${ - 4 * theme.gridUnit - }px`}; + padding: ${({ theme }) => `0 0 ${4 * theme.gridUnit}px`}; overflow: auto; } .field-length { @@ -247,9 +212,10 @@ export default function DataSourcePanel({ />
{t('Metrics')}} diff --git a/superset-frontend/src/explore/main.less b/superset-frontend/src/explore/main.less index eb568879f38d5..7dd47e115bcbd 100644 --- a/superset-frontend/src/explore/main.less +++ b/superset-frontend/src/explore/main.less @@ -224,7 +224,6 @@ h1.section-header { margin-bottom: 0; margin-top: 0; padding-bottom: 5px; - margin-left: -16px; } h2.section-header {