diff --git a/superset-frontend/src/components/Chart/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu.tsx index 784c6bdd6..c202d9ee9 100644 --- a/superset-frontend/src/components/Chart/ChartContextMenu.tsx +++ b/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 = ( , ); @@ -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, diff --git a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.tsx b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.tsx index 8269e8590..30f27ed81 100644 --- a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.tsx +++ b/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 }) => ( @@ -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 = ( - +
{filters.map((filter, i) => ( { + 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 +}); diff --git a/superset-frontend/src/components/Chart/utils.ts b/superset-frontend/src/components/Chart/utils.ts new file mode 100644 index 000000000..54fc5e892 --- /dev/null +++ b/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; +}