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

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;
+}