From 9780dc801a3eed1048d122454db5f290757db476 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 22 Oct 2020 18:06:25 +0200 Subject: [PATCH 01/10] Create Tooltip component --- .../src/common/components/Tooltip.tsx | 27 ++++++++++++++++ .../src/common/components/common.stories.tsx | 31 ++++++++++++++++++- 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 superset-frontend/src/common/components/Tooltip.tsx diff --git a/superset-frontend/src/common/components/Tooltip.tsx b/superset-frontend/src/common/components/Tooltip.tsx new file mode 100644 index 0000000000000..7e3c2a2f4d691 --- /dev/null +++ b/superset-frontend/src/common/components/Tooltip.tsx @@ -0,0 +1,27 @@ +/** + * 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 { Tooltip as BaseTooltip } from 'src/common/components'; +import { TooltipProps } from 'antd/lib/tooltip'; + +const Tooltip = (props: TooltipProps) => ( + +); + +export default Tooltip; diff --git a/superset-frontend/src/common/components/common.stories.tsx b/superset-frontend/src/common/components/common.stories.tsx index d142f2ac189d1..515f47fda6903 100644 --- a/superset-frontend/src/common/components/common.stories.tsx +++ b/superset-frontend/src/common/components/common.stories.tsx @@ -22,9 +22,10 @@ import { withKnobs, boolean, select } from '@storybook/addon-knobs'; import Modal from './Modal'; import Tabs, { EditableTabs } from './Tabs'; import AntdPopover from './Popover'; +import AntdTooltip from './Tooltip'; import { Menu } from '.'; import { Dropdown } from './Dropdown'; -import Button from '../../components/Button'; +import Button from 'src/components/Button'; export default { title: 'Common Components', @@ -146,3 +147,31 @@ export const Popover = () => ( ); + +export const Tooltip = () => ( + + + +); From c8ba7053921f0a2dc737f6e2471056d1f231dfea Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 22 Oct 2020 18:09:53 +0200 Subject: [PATCH 02/10] Refactor DatasourceControl --- .../components/controls/DatasourceControl.jsx | 103 ++++++++---------- 1 file changed, 47 insertions(+), 56 deletions(-) diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl.jsx index 932f1c0f800d9..424acaac8aa36 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl.jsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl.jsx @@ -18,21 +18,12 @@ */ import React from 'react'; import PropTypes from 'prop-types'; -import { - Col, - Collapse, - DropdownButton, - MenuItem, - OverlayTrigger, - Row, - Tooltip, - Well, -} from 'react-bootstrap'; +import { Col, Collapse, Row, Well } from 'react-bootstrap'; import { t, styled } from '@superset-ui/core'; import { ColumnOption, MetricOption } from '@superset-ui/chart-controls'; -import TooltipWrapper from 'src/components/TooltipWrapper'; - +import { Dropdown, Menu } from 'src/common/components'; +import Tooltip from 'src/common/components/Tooltip'; import Icon from 'src/components/Icon'; import ChangeDatasourceModal from 'src/datasource/ChangeDatasourceModal'; import DatasourceModal from 'src/datasource/DatasourceModal'; @@ -98,6 +89,7 @@ class DatasourceControl extends React.PureComponent { this.toggleEditDatasourceModal = this.toggleEditDatasourceModal.bind(this); this.toggleShowDatasource = this.toggleShowDatasource.bind(this); this.renderDatasource = this.renderDatasource.bind(this); + this.handleMenuItemClick = this.handleMenuItemClick.bind(this); } onDatasourceSave(datasource) { @@ -125,6 +117,15 @@ class DatasourceControl extends React.PureComponent { })); } + handleMenuItemClick({ key }) { + if (key === '1') { + this.toggleChangeDatasourceModal(); + } + if (key === '3') { + this.toggleEditDatasourceModal(); + } + } + renderDatasource() { const { datasource } = this.props; const { showDatasource } = this.state; @@ -176,18 +177,30 @@ class DatasourceControl extends React.PureComponent { showDatasource, } = this.state; const { datasource, onChange, value } = this.props; + + const datasourceMenu = ( + + {t('Change Dataset')} + + + {t('Explore in SQL Lab')} + + + + {t('Edit Dataset')} + + + ); + return (
- - {t('Expand/collapse dataset configuration')} - - } - > + - - + - } - className="" - bsSize="sm" - id="datasource_menu" - data-test="datasource-menu" - > - - {t('Change Dataset')} - - {datasource.type === 'table' && ( - - {t('Explore in SQL Lab')} - - )} - {this.props.isEditable && ( - - {t('Edit Dataset')} - - )} - - + + + +
{this.renderDatasource()} From d0038bb481c11debf2d340ef0d82a3ca46a3a5a3 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 22 Oct 2020 18:29:42 +0200 Subject: [PATCH 03/10] Bug fix --- .../components/DatasourceControl_spec.jsx | 12 ++++++++---- .../components/controls/DatasourceControl.jsx | 16 +++++++--------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx b/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx index 592fb975e77e2..3f6e6c6010eca 100644 --- a/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx @@ -20,7 +20,7 @@ import React from 'react'; import sinon from 'sinon'; import configureStore from 'redux-mock-store'; import { shallow } from 'enzyme'; -import { MenuItem } from 'react-bootstrap'; +import { Menu } from 'src/common/components'; import DatasourceModal from 'src/datasource/DatasourceModal'; import ChangeDatasourceModal from 'src/datasource/ChangeDatasourceModal'; import DatasourceControl from 'src/explore/components/controls/DatasourceControl'; @@ -73,15 +73,19 @@ describe('DatasourceControl', () => { it('show or hide Edit Datasource option', () => { let wrapper = setup(); expect(wrapper.find('#datasource_menu')).toExist(); - expect(wrapper.find('#datasource_menu').dive().find(MenuItem)).toHaveLength( - 3, + let menuWrapper = shallow( +
{wrapper.find('#datasource_menu').prop('overlay')}
, ); + expect(menuWrapper.find(Menu.Item)).toHaveLength(3); wrapper = setup({ isEditable: false, }); expect(wrapper.find('#datasource_menu')).toExist(); - expect(wrapper.find('#datasource_menu').dive().find(MenuItem)).toHaveLength( + menuWrapper = shallow( +
{wrapper.find('#datasource_menu').prop('overlay')}
, + ); + expect(menuWrapper.find(Menu.Item)).toHaveLength( 2, ); }); diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl.jsx index 424acaac8aa36..3e2e53e70ccee 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl.jsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl.jsx @@ -49,7 +49,7 @@ const defaultProps = { }; const Styles = styled.div` - #datasource_menu { + .ant-dropdown-trigger { margin-left: ${({ theme }) => theme.gridUnit}px; box-shadow: none; &:active { @@ -190,9 +190,11 @@ class DatasourceControl extends React.PureComponent { {t('Explore in SQL Lab')} - - {t('Edit Dataset')} - + {this.props.isEditable && ( + + {t('Edit Dataset')} + + )} ); @@ -220,11 +222,7 @@ class DatasourceControl extends React.PureComponent { data-test="datasource-menu" > - + From 5561a8038b2a41450ecda3432e78f1d9603c3968 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 22 Oct 2020 18:37:13 +0200 Subject: [PATCH 04/10] Lint fix --- .../javascripts/explore/components/DatasourceControl_spec.jsx | 4 +--- superset-frontend/src/common/components/common.stories.tsx | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx b/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx index 3f6e6c6010eca..d7811db95c495 100644 --- a/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx @@ -85,8 +85,6 @@ describe('DatasourceControl', () => { menuWrapper = shallow(
{wrapper.find('#datasource_menu').prop('overlay')}
, ); - expect(menuWrapper.find(Menu.Item)).toHaveLength( - 2, - ); + expect(menuWrapper.find(Menu.Item)).toHaveLength(2); }); }); diff --git a/superset-frontend/src/common/components/common.stories.tsx b/superset-frontend/src/common/components/common.stories.tsx index 515f47fda6903..36dd7c51a0330 100644 --- a/superset-frontend/src/common/components/common.stories.tsx +++ b/superset-frontend/src/common/components/common.stories.tsx @@ -19,13 +19,13 @@ import React from 'react'; import { action } from '@storybook/addon-actions'; import { withKnobs, boolean, select } from '@storybook/addon-knobs'; +import Button from 'src/components/Button'; import Modal from './Modal'; import Tabs, { EditableTabs } from './Tabs'; import AntdPopover from './Popover'; import AntdTooltip from './Tooltip'; import { Menu } from '.'; import { Dropdown } from './Dropdown'; -import Button from 'src/components/Button'; export default { title: 'Common Components', From 6ee0e242abacfe1655ee145118dbd7b6349bc059 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 22 Oct 2020 19:18:10 +0200 Subject: [PATCH 05/10] E2E test fix --- .../cypress/integration/explore/control.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 674974d7616b1..2cf6428bf786f 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts @@ -34,13 +34,13 @@ describe('Datasource control', () => { cy.visitChartByName('Num Births Trend'); cy.verifySliceSuccess({ waitAlias: '@postJson' }); - cy.get('#datasource_menu').click(); + cy.get('[data-test="datasource-menu"]').click(); cy.get('script').then(nodes => { numScripts = nodes.length; }); - cy.get('a').contains('Edit Dataset').click(); + cy.get('[data-test="edit-dataset"]').click(); // should load additional scripts for the modal cy.get('script').then(nodes => { @@ -65,8 +65,8 @@ describe('Datasource control', () => { .focus() .type(newMetricName, { force: true }); // delete metric - cy.get('#datasource_menu').click(); - cy.get('a').contains('Edit Dataset').click(); + cy.get('[data-test="datasource_menu"]').click(); + cy.get('[data-test="edit-dataset"]').click(); cy.get('.modal-content').within(() => { cy.get('a[role="tab"]').contains('Metrics').click(); }); From 764734f7c078eeb7afae55ad6f0f3bfa60a8c869 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 23 Oct 2020 13:54:51 +0200 Subject: [PATCH 06/10] Move menu item keys to constants --- .../components/controls/DatasourceControl.jsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl.jsx index 3e2e53e70ccee..7893d23e09e75 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl.jsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl.jsx @@ -75,6 +75,10 @@ const ColumnsCol = styled(Col)` } `; +const CHANGE_DATASET = 'change_dataset'; +const EXPLORE_IN_SQL_LAB = 'explore_in_sql_lab'; +const EDIT_DATASET = 'edit_dataset'; + class DatasourceControl extends React.PureComponent { constructor(props) { super(props); @@ -118,10 +122,10 @@ class DatasourceControl extends React.PureComponent { } handleMenuItemClick({ key }) { - if (key === '1') { + if (key === CHANGE_DATASET) { this.toggleChangeDatasourceModal(); } - if (key === '3') { + if (key === EDIT_DATASET) { this.toggleEditDatasourceModal(); } } @@ -180,8 +184,8 @@ class DatasourceControl extends React.PureComponent { const datasourceMenu = ( - {t('Change Dataset')} - + {t('Change Dataset')} + {this.props.isEditable && ( - + {t('Edit Dataset')} )} From 7a3864b3d1e815232fd5051391f217c6e15ec02a Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 23 Oct 2020 14:25:02 +0200 Subject: [PATCH 07/10] Remove LESS file --- .../components/controls/DatasourceControl.jsx | 19 ++++++-- .../controls/DatasourceControl.less | 48 ------------------- 2 files changed, 16 insertions(+), 51 deletions(-) delete mode 100644 superset-frontend/src/explore/components/controls/DatasourceControl.less diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl.jsx index 7893d23e09e75..777dbcc67a777 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl.jsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl.jsx @@ -30,7 +30,6 @@ import DatasourceModal from 'src/datasource/DatasourceModal'; import Label from 'src/components/Label'; import ControlHeader from '../ControlHeader'; -import './DatasourceControl.less'; const propTypes = { actions: PropTypes.object.isRequired, @@ -56,12 +55,23 @@ const Styles = styled.div` box-shadow: none; } } + .btn-group .open .dropdown-toggle { box-shadow: none; &.button-default { background: none; } } + + i.angle { + color: ${({ theme }) => theme.colors.primary.base}; + } + + svg.datasource-modal-trigger { + color: ${({ theme }) => theme.colors.primary.base}; + vertical-align: middle; + cursor: pointer; + } `; /** @@ -222,11 +232,14 @@ class DatasourceControl extends React.PureComponent { - + diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl.less b/superset-frontend/src/explore/components/controls/DatasourceControl.less deleted file mode 100644 index 571577ab75a9a..0000000000000 --- a/superset-frontend/src/explore/components/controls/DatasourceControl.less +++ /dev/null @@ -1,48 +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 '../../../../stylesheets/less/variables.less'; - -#datasource_menu { - border-radius: @border-radius-normal; - padding: 0px; - border: none; -} - -#datasource_menu .caret { - position: relative; - padding-right: 8px; - margin-left: 4px; - color: @lightest; - top: -8px; -} - -#datasource_menu .caret { - display: none; -} -#datasource_menu:hover { - background-color: transparent; -} -.DatasourceControl svg { - vertical-align: middle; - color: @brand-primary; - cursor: pointer; -} -.DatasourceControl .angle { - color: @brand-primary; -} From 6bdca35e7b7766ef8b7eed2ee486b4d975471f7e Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 23 Oct 2020 14:28:37 +0200 Subject: [PATCH 08/10] Test fix --- .../cypress-base/cypress/integration/explore/control.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 2cf6428bf786f..bbb2cd1d0732f 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts @@ -65,7 +65,7 @@ describe('Datasource control', () => { .focus() .type(newMetricName, { force: true }); // delete metric - cy.get('[data-test="datasource_menu"]').click(); + cy.get('[data-test="datasource-menu"]').click(); cy.get('[data-test="edit-dataset"]').click(); cy.get('.modal-content').within(() => { cy.get('a[role="tab"]').contains('Metrics').click(); From 018c714ed4158e218f499a46a5eec09708be62a4 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 23 Oct 2020 14:32:29 +0200 Subject: [PATCH 09/10] Test fix --- .../cypress/integration/explore/control.test.ts | 4 ++-- .../explore/components/DatasourceControl_spec.jsx | 8 ++++---- .../src/explore/components/controls/DatasourceControl.jsx | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) 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 bbb2cd1d0732f..70acb8d95d7a7 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts @@ -34,7 +34,7 @@ describe('Datasource control', () => { cy.visitChartByName('Num Births Trend'); cy.verifySliceSuccess({ waitAlias: '@postJson' }); - cy.get('[data-test="datasource-menu"]').click(); + cy.get('[data-test="datasource-menu-trigger"]').click(); cy.get('script').then(nodes => { numScripts = nodes.length; @@ -65,7 +65,7 @@ describe('Datasource control', () => { .focus() .type(newMetricName, { force: true }); // delete metric - cy.get('[data-test="datasource-menu"]').click(); + cy.get('[data-test="datasource-menu-trigger"]').click(); cy.get('[data-test="edit-dataset"]').click(); cy.get('.modal-content').within(() => { cy.get('a[role="tab"]').contains('Metrics').click(); diff --git a/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx b/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx index d7811db95c495..f8eaa43f45ac8 100644 --- a/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx @@ -72,18 +72,18 @@ describe('DatasourceControl', () => { it('show or hide Edit Datasource option', () => { let wrapper = setup(); - expect(wrapper.find('#datasource_menu')).toExist(); + expect(wrapper.find('[data-test="datasource-menu"]')).toExist(); let menuWrapper = shallow( -
{wrapper.find('#datasource_menu').prop('overlay')}
, +
{wrapper.find('[data-test="datasource-menu"]').prop('overlay')}
, ); expect(menuWrapper.find(Menu.Item)).toHaveLength(3); wrapper = setup({ isEditable: false, }); - expect(wrapper.find('#datasource_menu')).toExist(); + expect(wrapper.find('[data-test="datasource-menu"]')).toExist(); menuWrapper = shallow( -
{wrapper.find('#datasource_menu').prop('overlay')}
, +
{wrapper.find('[data-test="datasource-menu"]').prop('overlay')}
, ); expect(menuWrapper.find(Menu.Item)).toHaveLength(2); }); diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl.jsx index 777dbcc67a777..709a927c58274 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl.jsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl.jsx @@ -237,7 +237,7 @@ class DatasourceControl extends React.PureComponent { From 009bd79becb07c141b6fc928196cdeb84510ea6c Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 23 Oct 2020 15:03:28 +0200 Subject: [PATCH 10/10] Lint fix --- .../explore/components/DatasourceControl_spec.jsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx b/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx index f8eaa43f45ac8..8f6b8e9492c6a 100644 --- a/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx @@ -74,7 +74,9 @@ describe('DatasourceControl', () => { let wrapper = setup(); expect(wrapper.find('[data-test="datasource-menu"]')).toExist(); let menuWrapper = shallow( -
{wrapper.find('[data-test="datasource-menu"]').prop('overlay')}
, +
+ {wrapper.find('[data-test="datasource-menu"]').prop('overlay')} +
, ); expect(menuWrapper.find(Menu.Item)).toHaveLength(3); @@ -83,7 +85,9 @@ describe('DatasourceControl', () => { }); expect(wrapper.find('[data-test="datasource-menu"]')).toExist(); menuWrapper = shallow( -
{wrapper.find('[data-test="datasource-menu"]').prop('overlay')}
, +
+ {wrapper.find('[data-test="datasource-menu"]').prop('overlay')} +
, ); expect(menuWrapper.find(Menu.Item)).toHaveLength(2); });