fix: Popovers in Explore not attached to the fields they are triggered by

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;

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) => (
-      <Popover
+      <ControlPopover
         key={i}
         trigger="click"
-        placement="right"
         title={t('Edit annotation layer')}
         css={theme => ({
           '&:hover': {
@@ -190,17 +189,16 @@ class AnnotationLayerControl extends React.PureComponent {
           <span>{anno.name}</span>
           <span style={{ float: 'right' }}>{this.renderInfo(anno)}</span>
         </CustomListItem>
-      </Popover>
+      </ControlPopover>
     ));
 
     const addLayerPopoverKey = 'add';
     return (
       <div>
         <List bordered css={theme => ({ borderRadius: theme.gridUnit })}>
           {annotations}
-          <Popover
+          <ControlPopover
             trigger="click"
-            placement="right"
             content={this.renderPopover(addLayerPopoverKey, addedAnnotation)}
             title={t('Add annotation layer')}
             visible={this.state.popoverVisible[addLayerPopoverKey]}
@@ -216,7 +214,7 @@ class AnnotationLayerControl extends React.PureComponent {
               />{' '}
               &nbsp; {t('Add annotation layer')}
             </CustomListItem>
-          </Popover>
+          </ControlPopover>
         </List>
       </div>
     );

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<PopoverProps> => ({
+  trigger: 'click',
+  title: 'Control Popover Test',
+  content: <span data-test="control-popover-content">Information</span>,
+});
+
+const setupTest = (props: Partial<PopoverProps> = createProps()) => {
+  const setStateMock = jest.fn();
+  jest
+    .spyOn(React, 'useState')
+    .mockImplementation(((state: any) => [
+      state,
+      state === 'right' ? setStateMock : jest.fn(),
+    ]) as any);
+
+  const { container } = render(
+    <div data-test="outer-container">
+      <div id="controlSections">
+        <ControlPopover {...props}>
+          <span data-test="control-popover">Click me</span>
+        </ControlPopover>
+      </div>
+    </div>,
+  );
+
+  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');
+  });
+});

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<PopoverProps> = ({
+  getPopupContainer,
+  getVisibilityRatio = getElementYVisibilityRatioOnContainer,
+  ...props
+}) => {
+  const triggerElementRef = useRef<HTMLElement>();
+  const [placement, setPlacement] = React.useState<TooltipPlacement>('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 (
+    <Popover
+      {...props}
+      arrowPointAtCenter
+      placement={placement}
+      onVisibleChange={handleOnVisibleChange}
+      getPopupContainer={handleGetPopupContainer}
+    />
+  );
+};
+
+export default ControlPopover;

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;
 `;

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 (
-    <Popover
-      placement="right"
+    <ControlPopover
       trigger="click"
       content={overlayContent}
       defaultVisible={visible}
@@ -148,7 +147,7 @@ const ColumnSelectPopoverTrigger = ({
       destroyTooltipOnHide
     >
       {children}
-    </Popover>
+    </ControlPopover>
   );
 };
 

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', () => {