fix(dashboard): Ensure correct positioning of "Drill to detail by" su…

…bmenu (#21894)

superset-frontend/src/components/Chart/ChartContextMenu.tsx

@@ -36,9 +36,7 @@ import { findPermission } from 'src/utils/findPermission';
 import { Menu } from 'src/components/Menu';
 import { AntdDropdown as Dropdown } from 'src/components';
 import { DrillDetailMenuItems } from './DrillDetail';
-
-const MENU_ITEM_HEIGHT = 32;
-const MENU_VERTICAL_SPACING = 32;
+import { getMenuAdjustedY } from './utils';
 
 export interface ChartContextMenuProps {
   id: number;
@@ -78,8 +76,9 @@ const ChartContextMenu = (
       <DrillDetailMenuItems
         chartId={id}
         formData={formData}
-        isContextMenu
         filters={filters}
+        isContextMenu
+        contextMenuY={clientY}
         onSelection={onSelection}
       />,
     );
@@ -91,21 +90,12 @@ const ChartContextMenu = (
       clientY: number,
       filters?: BinaryQueryObjectFilterClause[],
     ) => {
-      // Viewport height
-      const vh = Math.max(
-        document.documentElement.clientHeight || 0,
-        window.innerHeight || 0,
-      );
-
       const itemsCount =
         [
           showDrillToDetail ? 2 : 0, // Drill to detail always has 2 top-level menu items
         ].reduce((a, b) => a + b, 0) || 1; // "No actions" appears if no actions in menu
 
-      const menuHeight = MENU_ITEM_HEIGHT * itemsCount + MENU_VERTICAL_SPACING;
-      // Always show the context menu inside the viewport
-      const adjustedY = vh - clientY < menuHeight ? vh - menuHeight : clientY;
-
+      const adjustedY = getMenuAdjustedY(clientY, itemsCount);
       setState({
         clientX,
         clientY: adjustedY,

superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.tsx

@@ -34,6 +34,9 @@ import { Menu } from 'src/components/Menu';
 import Icons from 'src/components/Icons';
 import { Tooltip } from 'src/components/Tooltip';
 import DrillDetailModal from './DrillDetailModal';
+import { getMenuAdjustedY, MENU_ITEM_HEIGHT } from '../utils';
+
+const MENU_PADDING = 4;
 
 const DisabledMenuItemTooltip = ({ title }: { title: ReactNode }) => (
   <Tooltip title={title} placement="top">
@@ -79,6 +82,7 @@ export type DrillDetailMenuItemsProps = {
   formData: QueryFormData;
   filters?: BinaryQueryObjectFilterClause[];
   isContextMenu?: boolean;
+  contextMenuY?: number;
   onSelection?: () => void;
   onClick?: (event: MouseEvent) => void;
 };
@@ -88,6 +92,7 @@ const DrillDetailMenuItems = ({
   formData,
   filters = [],
   isContextMenu = false,
+  contextMenuY = 0,
   onSelection = () => null,
   onClick = () => null,
   ...props
@@ -176,9 +181,22 @@ const DrillDetailMenuItems = ({
     );
   }
 
+  // Ensure submenu doesn't appear offscreen
+  const submenuYOffset = useMemo(() => {
+    const itemsCount = filters.length > 1 ? filters.length + 1 : filters.length;
+    const submenuY =
+      contextMenuY + MENU_PADDING + MENU_ITEM_HEIGHT + MENU_PADDING;
+
+    return getMenuAdjustedY(submenuY, itemsCount) - submenuY;
+  }, [contextMenuY, filters.length]);
+
   if (handlesDimensionContextMenu && !noAggregations && filters?.length) {
     drillToDetailByMenuItem = (
-      <Menu.SubMenu {...props} title={t('Drill to detail by')}>
+      <Menu.SubMenu
+        {...props}
+        popupOffset={[0, submenuYOffset]}
+        title={t('Drill to detail by')}
+      >
         <div data-test="drill-to-detail-by-submenu">
           {filters.map((filter, i) => (
             <Menu.Item

superset-frontend/src/components/Chart/utils.test.ts

@@ -0,0 +1,42 @@
+/**
+ * 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 { getMenuAdjustedY } from './utils';
+
+const originalInnerHeight = window.innerHeight;
+
+beforeEach(() => {
+  window.innerHeight = 500;
+});
+
+afterEach(() => {
+  window.innerHeight = originalInnerHeight;
+});
+
+test('correctly positions at upper edge of screen', () => {
+  expect(getMenuAdjustedY(75, 1)).toEqual(75); // No adjustment
+  expect(getMenuAdjustedY(75, 2)).toEqual(75); // No adjustment
+  expect(getMenuAdjustedY(75, 3)).toEqual(75); // No adjustment
+});
+
+test('correctly positions at lower edge of screen', () => {
+  expect(getMenuAdjustedY(425, 1)).toEqual(425); // No adjustment
+  expect(getMenuAdjustedY(425, 2)).toEqual(404); // Adjustment
+  expect(getMenuAdjustedY(425, 3)).toEqual(372); // Adjustment
+});

superset-frontend/src/components/Chart/utils.ts

@@ -0,0 +1,40 @@
+/**
+ * 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.
+ */
+
+export const MENU_ITEM_HEIGHT = 32;
+const MENU_VERTICAL_SPACING = 32;
+
+/**
+ * Calculates an adjusted Y-offset for a menu or submenu to prevent that
+ * menu from appearing offscreen
+ *
+ * @param clientY The original Y-offset
+ * @param itemsCount The number of menu items
+ */
+export function getMenuAdjustedY(clientY: number, itemsCount: number) {
+  // Viewport height
+  const vh = Math.max(
+    document.documentElement.clientHeight || 0,
+    window.innerHeight || 0,
+  );
+
+  const menuHeight = MENU_ITEM_HEIGHT * itemsCount + MENU_VERTICAL_SPACING;
+  // Always show the context menu inside the viewport
+  return vh - clientY < menuHeight ? vh - menuHeight : clientY;
+}