From dcdd334663e28d15368f8c454f2a36cbdb57332a Mon Sep 17 00:00:00 2001 From: Alexander Zobnin Date: Fri, 3 Nov 2023 10:06:51 +0100 Subject: [PATCH] RolePicker: Use portal to render menu (#77499) * RolePicker: Use portal for menu * Remove logging * Fix submenu styles * Fix ROLE_PICKER_MAX_MENU_WIDTH calculation * Fix first menu open glitch * Fix menu closing on ckick * Fix menu position --- .betterer.results | 3 - .../RolePicker/RoleMenuGroupsSection.tsx | 2 +- .../core/components/RolePicker/RolePicker.tsx | 92 ++++++++++++------- .../components/RolePicker/RolePickerMenu.tsx | 15 +-- .../core/components/RolePicker/constants.ts | 12 ++- .../app/core/components/RolePicker/styles.ts | 12 ++- 6 files changed, 86 insertions(+), 50 deletions(-) diff --git a/.betterer.results b/.betterer.results index bb7e73b4731..4b6eb1946a2 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1407,9 +1407,6 @@ exports[`better eslint`] = { [0, 0, 0, "Styles should be written using objects.", "5"], [0, 0, 0, "Styles should be written using objects.", "6"] ], - "public/app/core/components/RolePicker/RolePickerMenu.tsx:5381": [ - [0, 0, 0, "Styles should be written using objects.", "0"] - ], "public/app/core/components/RolePicker/ValueContainer.tsx:5381": [ [0, 0, 0, "Styles should be written using objects.", "0"] ], diff --git a/public/app/core/components/RolePicker/RoleMenuGroupsSection.tsx b/public/app/core/components/RolePicker/RoleMenuGroupsSection.tsx index 64c6cdc9e5d..340b392cb2d 100644 --- a/public/app/core/components/RolePicker/RoleMenuGroupsSection.tsx +++ b/public/app/core/components/RolePicker/RoleMenuGroupsSection.tsx @@ -26,7 +26,7 @@ interface RoleMenuGroupsSectionProps { selectedOptions: Role[]; onRoleChange: (option: Role) => void; onClearSubMenu: (group: string) => void; - showOnLeftSubMenu: boolean; + showOnLeftSubMenu?: boolean; } export const RoleMenuGroupsSection = React.forwardRef( diff --git a/public/app/core/components/RolePicker/RolePicker.tsx b/public/app/core/components/RolePicker/RolePicker.tsx index 2cd858380b5..9b38dead657 100644 --- a/public/app/core/components/RolePicker/RolePicker.tsx +++ b/public/app/core/components/RolePicker/RolePicker.tsx @@ -1,11 +1,11 @@ import React, { FormEvent, useCallback, useEffect, useState, useRef } from 'react'; -import { ClickOutsideWrapper, useTheme2 } from '@grafana/ui'; +import { ClickOutsideWrapper, Portal, useTheme2 } from '@grafana/ui'; import { Role, OrgRole } from 'app/types'; import { RolePickerInput } from './RolePickerInput'; import { RolePickerMenu } from './RolePickerMenu'; -import { MENU_MAX_HEIGHT, ROLE_PICKER_SUBMENU_MIN_WIDTH, ROLE_PICKER_WIDTH } from './constants'; +import { MENU_MAX_HEIGHT, ROLE_PICKER_MAX_MENU_WIDTH, ROLE_PICKER_WIDTH } from './constants'; export interface Props { basicRole?: OrgRole; @@ -48,6 +48,7 @@ export const RolePicker = ({ const [selectedBuiltInRole, setSelectedBuiltInRole] = useState(basicRole); const [query, setQuery] = useState(''); const [offset, setOffset] = useState({ vertical: 0, horizontal: 0 }); + const [menuLeft, setMenuLeft] = useState(false); const ref = useRef(null); const theme = useTheme2(); const widthPx = typeof width === 'number' ? theme.spacing(width) : width; @@ -57,22 +58,36 @@ export const RolePicker = ({ setSelectedRoles(appliedRoles); }, [appliedRoles, basicRole, onBasicRoleChange]); + const setMenuPosition = useCallback(() => { + const { horizontal, vertical, menuToLeft } = calculateMenuPosition(); + if (horizontal && vertical) { + setOffset({ horizontal, vertical }); + setMenuLeft(menuToLeft); + } + }, []); + useEffect(() => { - const dimensions = ref?.current?.getBoundingClientRect(); - if (!dimensions || !isOpen) { + if (!isOpen) { return; } - const { bottom, top, left, right, width: currentRolePickerWidth } = dimensions; - const distance = window.innerHeight - bottom; - const offsetVertical = bottom - top + 10; // Add extra 10px to offset to account for border and outline - const offsetHorizontal = right - left; - let horizontal = -offsetHorizontal; - let vertical = -offsetVertical; + setMenuPosition(); + }, [isOpen, selectedRoles, setMenuPosition]); + const calculateMenuPosition = () => { + const dimensions = ref?.current?.getBoundingClientRect(); + if (!dimensions) { + return {}; + } + const { bottom, top, left, right } = dimensions; + let horizontal = left; + let vertical = bottom + 10; // Add extra 10px to offset to account for border and outline + let menuToLeft = false; + + const distance = window.innerHeight - bottom; if (distance < MENU_MAX_HEIGHT + 20) { // Off set to display the role picker menu at the bottom of the screen // without resorting to scroll the page - vertical = 50 + (MENU_MAX_HEIGHT - distance) - offsetVertical; + vertical = top - MENU_MAX_HEIGHT - 50; } /* @@ -82,25 +97,24 @@ export const RolePicker = ({ * both (the role picker menu and its sub menu) aligned to the left edge of the input. * Otherwise, it aligns the role picker menu to the right. */ - if ( - window.innerWidth - right < currentRolePickerWidth && - currentRolePickerWidth < 2 * ROLE_PICKER_SUBMENU_MIN_WIDTH - ) { - horizontal = offsetHorizontal; + if (left + ROLE_PICKER_MAX_MENU_WIDTH > window.innerWidth) { + horizontal = window.innerWidth - right; + menuToLeft = true; } - setOffset({ horizontal, vertical }); - }, [isOpen, selectedRoles]); + return { horizontal, vertical, menuToLeft }; + }; const onOpen = useCallback( (event: FormEvent) => { if (!disabled) { event.preventDefault(); event.stopPropagation(); + setMenuPosition(); setOpen(true); } }, - [setOpen, disabled] + [disabled, setMenuPosition] ); const onClose = useCallback(() => { @@ -160,7 +174,7 @@ export const RolePicker = ({ }} ref={ref} > - + {isOpen && ( - + + {/* Since menu rendered in portal and whole component wrapped in ClickOutsideWrapper, */} + {/* we need to stop event propagation to prevent closing menu */} + {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} +
e.stopPropagation()}> + +
+ )} diff --git a/public/app/core/components/RolePicker/RolePickerMenu.tsx b/public/app/core/components/RolePicker/RolePickerMenu.tsx index 1da2a057b52..1bfe1d63ce7 100644 --- a/public/app/core/components/RolePicker/RolePickerMenu.tsx +++ b/public/app/core/components/RolePicker/RolePickerMenu.tsx @@ -63,6 +63,7 @@ interface RolePickerMenuProps { updateDisabled?: boolean; apply?: boolean; offset: { vertical: number; horizontal: number }; + menuLeft?: boolean; } export const RolePickerMenu = ({ @@ -78,6 +79,7 @@ export const RolePickerMenu = ({ onUpdate, updateDisabled, offset, + menuLeft, apply, }: RolePickerMenuProps): JSX.Element => { const [selectedOptions, setSelectedOptions] = useState(appliedRoles); @@ -206,11 +208,12 @@ export const RolePickerMenu = ({ className={cx( styles.menu, customStyles.menuWrapper, - { [customStyles.menuLeft]: offset.horizontal > 0 }, - css` - bottom: ${offset.vertical > 0 ? `${offset.vertical}px` : 'unset'}; - top: ${offset.vertical < 0 ? `${Math.abs(offset.vertical)}px` : 'unset'}; - ` + { [customStyles.menuLeft]: menuLeft }, + css({ + top: `${offset.vertical}px`, + left: !menuLeft ? `${offset.horizontal}px` : 'unset', + right: menuLeft ? `${offset.horizontal}px` : 'unset', + }) )} >
@@ -248,7 +251,7 @@ export const RolePickerMenu = ({ selectedOptions={selectedOptions} onRoleChange={onChange} onClearSubMenu={onClearSubMenu} - showOnLeftSubMenu={offset.horizontal > 0} + showOnLeftSubMenu={menuLeft} /> ))} diff --git a/public/app/core/components/RolePicker/constants.ts b/public/app/core/components/RolePicker/constants.ts index 3b6c35447da..f2c0f8818fa 100644 --- a/public/app/core/components/RolePicker/constants.ts +++ b/public/app/core/components/RolePicker/constants.ts @@ -1,3 +1,11 @@ -export const MENU_MAX_HEIGHT = 300; // max height for the picker's dropdown menu export const ROLE_PICKER_WIDTH = 360; -export const ROLE_PICKER_SUBMENU_MIN_WIDTH = 260; + +export const MENU_MAX_HEIGHT = 300; // max height for the picker's dropdown menu + +export const ROLE_PICKER_MENU_MIN_WIDTH = 320; +export const ROLE_PICKER_MENU_MAX_WIDTH = 360; + +export const ROLE_PICKER_SUBMENU_MIN_WIDTH = 320; +export const ROLE_PICKER_SUBMENU_MAX_WIDTH = 360; + +export const ROLE_PICKER_MAX_MENU_WIDTH = ROLE_PICKER_MENU_MAX_WIDTH + ROLE_PICKER_SUBMENU_MAX_WIDTH; diff --git a/public/app/core/components/RolePicker/styles.ts b/public/app/core/components/RolePicker/styles.ts index 74ce953af80..befe0602a92 100644 --- a/public/app/core/components/RolePicker/styles.ts +++ b/public/app/core/components/RolePicker/styles.ts @@ -2,7 +2,12 @@ import { css } from '@emotion/css'; import { GrafanaTheme2 } from '@grafana/data'; -import { ROLE_PICKER_SUBMENU_MIN_WIDTH } from './constants'; +import { + ROLE_PICKER_MENU_MAX_WIDTH, + ROLE_PICKER_MENU_MIN_WIDTH, + ROLE_PICKER_SUBMENU_MAX_WIDTH, + ROLE_PICKER_SUBMENU_MIN_WIDTH, +} from './constants'; export const getStyles = (theme: GrafanaTheme2) => ({ hideScrollBar: css({ @@ -24,18 +29,19 @@ export const getStyles = (theme: GrafanaTheme2) => ({ minWidth: 'auto', }), menu: css({ - minWidth: `${ROLE_PICKER_SUBMENU_MIN_WIDTH}px`, + minWidth: `${ROLE_PICKER_MENU_MIN_WIDTH}px`, + maxWidth: `${ROLE_PICKER_MENU_MAX_WIDTH}px`, '& > div': { paddingTop: theme.spacing(1), }, }), menuLeft: css({ - right: 0, flexDirection: 'row-reverse', }), subMenu: css({ height: '100%', minWidth: `${ROLE_PICKER_SUBMENU_MIN_WIDTH}px`, + maxWidth: `${ROLE_PICKER_SUBMENU_MAX_WIDTH}px`, display: 'flex', flexDirection: 'column', borderLeft: `1px solid ${theme.components.input.borderColor}`,