diff --git a/superset-frontend/src/components/Popover/index.tsx b/superset-frontend/src/components/Popover/index.tsx index 880e45791..bccc31c35 100644 --- a/superset-frontend/src/components/Popover/index.tsx +++ b/superset-frontend/src/components/Popover/index.tsx @@ -18,6 +18,9 @@ */ import { Popover } from 'antd'; +export { PopoverProps } from 'antd/lib/popover'; +export { TooltipPlacement } from 'antd/lib/tooltip'; + // Eventually Popover can be wrapped and customized in this file // for now we're just redirecting export default Popover; diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.jsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.jsx index aa3255374..06fb582b5 100644 --- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.jsx +++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.jsx @@ -22,11 +22,11 @@ import { List } from 'src/components'; import { connect } from 'react-redux'; import { t, withTheme } from '@superset-ui/core'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; -import Popover from 'src/components/Popover'; import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; import { getChartKey } from 'src/explore/exploreUtils'; import { runAnnotationQuery } from 'src/components/Chart/chartAction'; import CustomListItem from 'src/explore/components/controls/CustomListItem'; +import ControlPopover from '../ControlPopover/ControlPopover'; const AnnotationLayer = AsyncEsmComponent( () => import('./AnnotationLayer'), @@ -167,10 +167,9 @@ class AnnotationLayerControl extends React.PureComponent { const addedAnnotation = this.props.value[addedAnnotationIndex]; const annotations = this.props.value.map((anno, i) => ( - ({ '&:hover': { @@ -190,7 +189,7 @@ class AnnotationLayerControl extends React.PureComponent { {anno.name} {this.renderInfo(anno)} - + )); const addLayerPopoverKey = 'add'; @@ -198,9 +197,8 @@ class AnnotationLayerControl extends React.PureComponent {
({ borderRadius: theme.gridUnit })}> {annotations} - {' '}   {t('Add annotation layer')} - +
); diff --git a/superset-frontend/src/explore/components/controls/ControlPopover/ControlPopover.test.tsx b/superset-frontend/src/explore/components/controls/ControlPopover/ControlPopover.test.tsx new file mode 100644 index 000000000..de14f78f7 --- /dev/null +++ b/superset-frontend/src/explore/components/controls/ControlPopover/ControlPopover.test.tsx @@ -0,0 +1,126 @@ +/** + * 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 { render, screen } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; +import { waitFor } from '@testing-library/react'; + +import ControlPopover, { PopoverProps } from './ControlPopover'; + +const createProps = (): Partial => ({ + trigger: 'click', + title: 'Control Popover Test', + content: Information, +}); + +const setupTest = (props: Partial = createProps()) => { + const setStateMock = jest.fn(); + jest + .spyOn(React, 'useState') + .mockImplementation(((state: any) => [ + state, + state === 'right' ? setStateMock : jest.fn(), + ]) as any); + + const { container } = render( +
+
+ + Click me + +
+
, + ); + + return { + props, + container, + setStateMock, + }; +}; + +afterEach(() => { + jest.restoreAllMocks(); +}); + +test('Should render', () => { + setupTest(); + expect(screen.getByTestId('control-popover')).toBeInTheDocument(); + userEvent.click(screen.getByTestId('control-popover')); + expect(screen.getByText('Control Popover Test')).toBeInTheDocument(); + expect(screen.getByTestId('control-popover-content')).toBeInTheDocument(); +}); + +test('Should lock the vertical scroll when the popup is visible', () => { + setupTest(); + expect(screen.getByTestId('control-popover')).toBeInTheDocument(); + expect(screen.getByTestId('outer-container')).not.toHaveStyle( + 'overflowY: hidden', + ); + userEvent.click(screen.getByTestId('control-popover')); + expect(screen.getByTestId('outer-container')).toHaveStyle( + 'overflowY: hidden', + ); + userEvent.click(document.body); + expect(screen.getByTestId('outer-container')).not.toHaveStyle( + 'overflowY: hidden', + ); +}); + +test('Should place popup at the top', async () => { + const { setStateMock } = setupTest({ + ...createProps(), + getVisibilityRatio: () => 0.2, + }); + + expect(screen.getByTestId('control-popover')).toBeInTheDocument(); + userEvent.click(screen.getByTestId('control-popover')); + + await waitFor(() => { + expect(setStateMock).toHaveBeenCalledWith('rightTop'); + }); +}); + +test('Should place popup at the center', async () => { + const { setStateMock } = setupTest({ + ...createProps(), + getVisibilityRatio: () => 0.5, + }); + + expect(screen.getByTestId('control-popover')).toBeInTheDocument(); + userEvent.click(screen.getByTestId('control-popover')); + + await waitFor(() => { + expect(setStateMock).toHaveBeenCalledWith('right'); + }); +}); + +test('Should place popup at the bottom', async () => { + const { setStateMock } = setupTest({ + ...createProps(), + getVisibilityRatio: () => 0.7, + }); + + expect(screen.getByTestId('control-popover')).toBeInTheDocument(); + userEvent.click(screen.getByTestId('control-popover')); + + await waitFor(() => { + expect(setStateMock).toHaveBeenCalledWith('rightBottom'); + }); +}); diff --git a/superset-frontend/src/explore/components/controls/ControlPopover/ControlPopover.tsx b/superset-frontend/src/explore/components/controls/ControlPopover/ControlPopover.tsx new file mode 100644 index 000000000..70ade69f2 --- /dev/null +++ b/superset-frontend/src/explore/components/controls/ControlPopover/ControlPopover.tsx @@ -0,0 +1,118 @@ +/** + * 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, { useCallback, useRef, useEffect } from 'react'; + +import Popover, { + PopoverProps as BasePopoverProps, + TooltipPlacement, +} from 'src/components/Popover'; + +const sectionContainerId = 'controlSections'; +const getSectionContainerElement = () => + document.getElementById(sectionContainerId)?.parentElement; + +const getElementYVisibilityRatioOnContainer = (node: HTMLElement) => { + const containerHeight = window?.innerHeight; + const nodePositionInViewport = node.getBoundingClientRect()?.top; + if (!containerHeight || !nodePositionInViewport) { + return 0; + } + + return nodePositionInViewport / containerHeight; +}; + +export type PopoverProps = BasePopoverProps & { + getVisibilityRatio?: typeof getElementYVisibilityRatioOnContainer; +}; + +const ControlPopover: React.FC = ({ + getPopupContainer, + getVisibilityRatio = getElementYVisibilityRatioOnContainer, + ...props +}) => { + const triggerElementRef = useRef(); + const [placement, setPlacement] = React.useState('right'); + + const calculatePlacement = useCallback(() => { + const visibilityRatio = getVisibilityRatio(triggerElementRef.current!); + + if (visibilityRatio < 0.35) { + setPlacement('rightTop'); + } else if (visibilityRatio > 0.65) { + setPlacement('rightBottom'); + } else { + setPlacement('right'); + } + }, [getVisibilityRatio]); + + const changeContainerScrollStatus = useCallback( + visible => { + if (triggerElementRef.current && visible) { + calculatePlacement(); + } + + const element = getSectionContainerElement(); + if (element) { + element.style.overflowY = visible ? 'hidden' : 'auto'; + } + }, + [calculatePlacement], + ); + + const handleGetPopupContainer = useCallback( + (triggerNode: HTMLElement) => { + triggerElementRef.current = triggerNode; + setTimeout(() => { + calculatePlacement(); + }, 0); + + return getPopupContainer?.(triggerNode) || document.body; + }, + [calculatePlacement, getPopupContainer], + ); + + const handleOnVisibleChange = useCallback( + (visible: boolean) => { + if (props.visible === undefined) { + changeContainerScrollStatus(visible); + } + + props.onVisibleChange?.(visible); + }, + [props, changeContainerScrollStatus], + ); + + useEffect(() => { + if (props.visible !== undefined) { + changeContainerScrollStatus(props.visible); + } + }, [props.visible, changeContainerScrollStatus]); + + return ( + + ); +}; + +export default ControlPopover; diff --git a/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx b/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx index 24ee1f870..682a961ec 100644 --- a/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx +++ b/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx @@ -31,7 +31,6 @@ import { getClientErrorObject } from 'src/utils/getClientErrorObject'; import Button from 'src/components/Button'; import ControlHeader from 'src/explore/components/ControlHeader'; import Label, { Type } from 'src/components/Label'; -import Popover from 'src/components/Popover'; import { Divider } from 'src/components'; import Icons from 'src/components/Icons'; import Select from 'src/components/Select/Select'; @@ -42,6 +41,7 @@ import { SLOW_DEBOUNCE } from 'src/constants'; import { testWithId } from 'src/utils/testUtils'; import { noOp } from 'src/utils/common'; import { FrameType } from './types'; +import ControlPopover from '../ControlPopover/ControlPopover'; import { CommonFrame, @@ -86,7 +86,7 @@ const fetchTimeRange = async (timeRange: string) => { } }; -const StyledPopover = styled(Popover)``; +const StyledPopover = styled(ControlPopover)``; const StyledRangeType = styled(Select)` width: 272px; `; diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopoverTrigger.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopoverTrigger.tsx index 42251464d..62107753e 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopoverTrigger.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopoverTrigger.tsx @@ -23,10 +23,10 @@ import { isAdhocColumn, isColumnMeta, } from '@superset-ui/chart-controls'; -import Popover from 'src/components/Popover'; import { ExplorePopoverContent } from 'src/explore/components/ExploreContentPopover'; import ColumnSelectPopover from './ColumnSelectPopover'; import { DndColumnSelectPopoverTitle } from './DndColumnSelectPopoverTitle'; +import ControlPopover from '../ControlPopover/ControlPopover'; interface ColumnSelectPopoverTriggerProps { columns: ColumnMeta[]; @@ -137,8 +137,7 @@ const ColumnSelectPopoverTrigger = ({ ); return ( - {children} - + ); }; diff --git a/superset-frontend/src/explore/components/controls/FilterBoxItemControl/FilterBoxItemControl.test.jsx b/superset-frontend/src/explore/components/controls/FilterBoxItemControl/FilterBoxItemControl.test.jsx index 597c4446e..4cf88645d 100644 --- a/superset-frontend/src/explore/components/controls/FilterBoxItemControl/FilterBoxItemControl.test.jsx +++ b/superset-frontend/src/explore/components/controls/FilterBoxItemControl/FilterBoxItemControl.test.jsx @@ -21,10 +21,10 @@ import React from 'react'; import sinon from 'sinon'; import { shallow } from 'enzyme'; -import Popover from 'src/components/Popover'; import FilterBoxItemControl from 'src/explore/components/controls/FilterBoxItemControl'; import FormRow from 'src/components/FormRow'; import datasources from 'spec/fixtures/mockDatasource'; +import ControlPopover from '../ControlPopover/ControlPopover'; const defaultProps = { label: 'some label', @@ -46,7 +46,7 @@ describe('FilterBoxItemControl', () => { }); it('renders a Popover', () => { - expect(wrapper.find(Popover)).toExist(); + expect(wrapper.find(ControlPopover)).toExist(); }); it('renderForms does the job', () => { diff --git a/superset-frontend/src/explore/components/controls/FilterBoxItemControl/FilterBoxItemControl.test.tsx b/superset-frontend/src/explore/components/controls/FilterBoxItemControl/FilterBoxItemControl.test.tsx index 9bac8d253..4ad09e958 100644 --- a/superset-frontend/src/explore/components/controls/FilterBoxItemControl/FilterBoxItemControl.test.tsx +++ b/superset-frontend/src/explore/components/controls/FilterBoxItemControl/FilterBoxItemControl.test.tsx @@ -38,14 +38,14 @@ const createProps = () => ({ onChange: jest.fn(), }); -test('Shoud render', () => { +test('Should render', () => { const props = createProps(); render(); expect(screen.getByTestId('FilterBoxItemControl')).toBeInTheDocument(); expect(screen.getByRole('button')).toBeInTheDocument(); }); -test('Shoud open modal', () => { +test('Should open modal', () => { const props = createProps(); render(); userEvent.click(screen.getByRole('button')); diff --git a/superset-frontend/src/explore/components/controls/FilterBoxItemControl/index.jsx b/superset-frontend/src/explore/components/controls/FilterBoxItemControl/index.jsx index 25aa79f44..4c8a367e3 100644 --- a/superset-frontend/src/explore/components/controls/FilterBoxItemControl/index.jsx +++ b/superset-frontend/src/explore/components/controls/FilterBoxItemControl/index.jsx @@ -21,12 +21,12 @@ import PropTypes from 'prop-types'; import { t } from '@superset-ui/core'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; -import Popover from 'src/components/Popover'; import FormRow from 'src/components/FormRow'; import { Select } from 'src/components'; import CheckboxControl from 'src/explore/components/controls/CheckboxControl'; import TextControl from 'src/explore/components/controls/TextControl'; import { FILTER_CONFIG_ATTRIBUTES } from 'src/explore/constants'; +import ControlPopover from '../ControlPopover/ControlPopover'; const INTEGRAL_TYPES = new Set([ 'TINYINT', @@ -275,9 +275,8 @@ export default class FilterBoxItemControl extends React.Component { return ( {this.textSummary()}{' '} - @@ -286,7 +285,7 @@ export default class FilterBoxItemControl extends React.Component { className="text-primary" label="edit-ts-column" /> - + ); } diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterPopoverTrigger/AdhocFilterPopoverTrigger.test.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterPopoverTrigger/AdhocFilterPopoverTrigger.test.tsx index 92d0ca801..36c711ebf 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterPopoverTrigger/AdhocFilterPopoverTrigger.test.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterPopoverTrigger/AdhocFilterPopoverTrigger.test.tsx @@ -73,7 +73,8 @@ test('should be visible when controlled', async () => { Click , ); - expect(screen.getByRole('tooltip')).toBeInTheDocument(); + + expect(await screen.findByRole('tooltip')).toBeInTheDocument(); }); test('should NOT be visible when controlled', () => { diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterPopoverTrigger/index.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterPopoverTrigger/index.tsx index 15913e65c..5dd6993c1 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterPopoverTrigger/index.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterPopoverTrigger/index.tsx @@ -17,12 +17,12 @@ * under the License. */ import React from 'react'; -import Popover from 'src/components/Popover'; import { OptionSortType } from 'src/explore/types'; import AdhocFilterEditPopover from 'src/explore/components/controls/FilterControl/AdhocFilterEditPopover'; import AdhocFilter from 'src/explore/components/controls/FilterControl/AdhocFilter'; import { ExplorePopoverContent } from 'src/explore/components/ExploreContentPopover'; import { Operators } from 'src/explore/constants'; +import ControlPopover from '../../ControlPopover/ControlPopover'; interface AdhocFilterPopoverTriggerProps { sections?: string[]; @@ -101,8 +101,7 @@ class AdhocFilterPopoverTrigger extends React.PureComponent< ); return ( - {this.props.children} - + ); } } diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.test.jsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.test.jsx index 14b4185ab..3700bb083 100644 --- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.test.jsx +++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.test.jsx @@ -21,10 +21,10 @@ import React from 'react'; import sinon from 'sinon'; import { shallow } from 'enzyme'; -import Popover from 'src/components/Popover'; import { AGGREGATES } from 'src/explore/constants'; import AdhocMetricOption from 'src/explore/components/controls/MetricControl/AdhocMetricOption'; import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric'; +import ControlPopover from '../ControlPopover/ControlPopover'; const columns = [ { type: 'VARCHAR(255)', column_name: 'source' }, @@ -59,7 +59,7 @@ function setup(overrides) { describe('AdhocMetricOption', () => { it('renders an overlay trigger wrapper for the label', () => { const { wrapper } = setup(); - expect(wrapper.find(Popover)).toExist(); + expect(wrapper.find(ControlPopover)).toExist(); expect(wrapper.find('OptionControlLabel')).toExist(); }); diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx index 42c2c806b..273c83e7d 100644 --- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx +++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx @@ -18,7 +18,6 @@ */ import React, { ReactNode } from 'react'; import { Datasource, Metric } from '@superset-ui/core'; -import Popover from 'src/components/Popover'; import AdhocMetricEditPopoverTitle from 'src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle'; import { ExplorePopoverContent } from 'src/explore/components/ExploreContentPopover'; import AdhocMetricEditPopover, { @@ -26,6 +25,7 @@ import AdhocMetricEditPopover, { } from './AdhocMetricEditPopover'; import AdhocMetric from './AdhocMetric'; import { savedMetricType } from './types'; +import ControlPopover from '../ControlPopover/ControlPopover'; export type AdhocMetricPopoverTriggerProps = { adhocMetric: AdhocMetric; @@ -223,7 +223,7 @@ class AdhocMetricPopoverTrigger extends React.PureComponent< ); return ( - {this.props.children} - + ); } } diff --git a/superset-frontend/src/explore/components/controls/TimeSeriesColumnControl/index.jsx b/superset-frontend/src/explore/components/controls/TimeSeriesColumnControl/index.jsx index 29d0d16b6..5070c9a59 100644 --- a/superset-frontend/src/explore/components/controls/TimeSeriesColumnControl/index.jsx +++ b/superset-frontend/src/explore/components/controls/TimeSeriesColumnControl/index.jsx @@ -20,12 +20,12 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Input } from 'src/components/Input'; import Button from 'src/components/Button'; -import Popover from 'src/components/Popover'; import { Select, Row, Col } from 'src/components'; import { t, styled } from '@superset-ui/core'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import BoundsControl from '../BoundsControl'; import CheckboxControl from '../CheckboxControl'; +import ControlPopover from '../ControlPopover/ControlPopover'; const propTypes = { label: PropTypes.string, @@ -353,9 +353,8 @@ export default class TimeSeriesColumnControl extends React.Component { return ( {this.textSummary()}{' '} - - + ); }