You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2022/10/21 11:49:41 UTC
[superset] branch master updated: fix(dashboard): Ensure correct positioning of "Drill to detail by" submenu (#21894)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 40f82545ab fix(dashboard): Ensure correct positioning of "Drill to detail by" submenu (#21894)
40f82545ab is described below
commit 40f82545abf385f24a8681b6655ceb5042e1f3a7
Author: Cody Leff <co...@preset.io>
AuthorDate: Fri Oct 21 05:49:31 2022 -0600
fix(dashboard): Ensure correct positioning of "Drill to detail by" submenu (#21894)
---
.../src/components/Chart/ChartContextMenu.tsx | 18 +++-------
.../Chart/DrillDetail/DrillDetailMenuItems.tsx | 20 ++++++++++-
.../src/components/Chart/utils.test.ts | 42 ++++++++++++++++++++++
superset-frontend/src/components/Chart/utils.ts | 40 +++++++++++++++++++++
4 files changed, 105 insertions(+), 15 deletions(-)
diff --git a/superset-frontend/src/components/Chart/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu.tsx
index 784c6bdd65..c202d9ee9d 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 = (
<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,
diff --git a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.tsx b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.tsx
index 8269e85900..30f27ed818 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 }) => (
<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
diff --git a/superset-frontend/src/components/Chart/utils.test.ts b/superset-frontend/src/components/Chart/utils.test.ts
new file mode 100644
index 0000000000..b8de155a2a
--- /dev/null
+++ b/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
+});
diff --git a/superset-frontend/src/components/Chart/utils.ts b/superset-frontend/src/components/Chart/utils.ts
new file mode 100644
index 0000000000..54fc5e8926
--- /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;
+}