fix(flamegraph): fix dropdown menu opening (#1755)

grafana · Nov 23, 2022 · 0b1acef · 0b1acef
1 parent b271373
commit 0b1acef
Show file tree

Hide file tree

Showing 15 changed files with 46 additions and 54 deletions.
diff --git a/package.json b/package.json
@@ -180,7 +180,7 @@
     "@react-hook/resize-observer": "^1.2.4",
     "@react-hook/window-size": "^3.0.7",
     "@reduxjs/toolkit": "^1.6.2",
-    "@szhsin/react-menu": "^1.10.1",
+    "@szhsin/react-menu": "3.3.0",
     "@types/copy-webpack-plugin": "^6.0.0",
     "@types/react": "^17.0.0",
     "@types/react-notifications-component": "^3.1.1",

diff --git a/packages/pyroscope-flamegraph/src/FlameGraph/FlameGraphComponent/ContextMenu.spec.tsx b/packages/pyroscope-flamegraph/src/FlameGraph/FlameGraphComponent/ContextMenu.spec.tsx
@@ -1,7 +1,7 @@
 /* eslint-disable react/jsx-props-no-spreading */
 import React from 'react';
 import { render, screen } from '@testing-library/react';
-import { MenuItem } from '@szhsin/react-menu';
+import { MenuItem } from '@webapp/ui/Menu';
 import userEvent from '@testing-library/user-event';
 
 import ContextMenu, { ContextMenuProps } from './ContextMenu';

diff --git a/packages/pyroscope-flamegraph/src/FlameGraph/FlameGraphComponent/ContextMenu.tsx b/packages/pyroscope-flamegraph/src/FlameGraph/FlameGraphComponent/ContextMenu.tsx
@@ -1,5 +1,6 @@
+/* eslint-disable react/jsx-props-no-spreading, import/no-extraneous-dependencies */
 import React from 'react';
-import { ControlledMenu, useMenuState } from '@szhsin/react-menu';
+import { ControlledMenu, useMenuState } from '@webapp/ui/Menu';
 import styles from './ContextMenu.module.scss';
 
 type xyToMenuItems = (x: number, y: number) => JSX.Element[];
@@ -19,7 +20,7 @@ export interface ContextMenuProps {
 }
 
 export default function ContextMenu(props: ContextMenuProps) {
-  const { openMenu, closeMenu, ...menuProps } = useMenuState(false);
+  const [menuProps, toggleMenu] = useMenuState({ transition: true });
   const [anchorPoint, setAnchorPoint] = React.useState({ x: 0, y: 0 });
   const { canvasRef } = props;
   const [menuItems, setMenuItems] = React.useState<JSX.Element[]>([]);
@@ -30,13 +31,13 @@ export default function ContextMenu(props: ContextMenuProps) {
   } = props;
 
   const onClose = () => {
-    closeMenu();
+    toggleMenu(false);
 
     onCloseCallback();
   };
 
   React.useEffect(() => {
-    closeMenu();
+    toggleMenu(false);
 
     // use closure to "cache" the current canvas reference
     // so that when cleaning up, it points to a valid canvas
@@ -52,15 +53,13 @@ export default function ContextMenu(props: ContextMenuProps) {
       const items = xyToMenuItems(e.offsetX, e.offsetY);
       setMenuItems(items);
 
-      //      console.log('set menu items', items);
-
       // TODO
       // if the menu becomes too large, it may overflow to outside the screen
       const x = e.clientX;
       const y = e.clientY + 20;
 
       setAnchorPoint({ x, y });
-      openMenu();
+      toggleMenu(true);
 
       onOpenCallback(e.offsetX, e.offsetY);
     };
@@ -75,10 +74,8 @@ export default function ContextMenu(props: ContextMenuProps) {
 
   return (
     <ControlledMenu
+      {...menuProps}
       className={styles.dummy}
-      menuItemFocus={menuProps.menuItemFocus}
-      isMounted={menuProps.isMounted}
-      isOpen={menuProps.isOpen}
       anchorPoint={anchorPoint}
       onClose={onClose}
     >

diff --git a/...oscope-flamegraph/src/FlameGraph/FlameGraphComponent/DiffLegendPaletteDropdown.module.css b/...oscope-flamegraph/src/FlameGraph/FlameGraphComponent/DiffLegendPaletteDropdown.module.css
@@ -25,10 +25,6 @@
   display: flex;
 }
 
-.diffPaletteDropdown ::global(.rc-menu-item) {
-  display: flex;
-}
-
 .dropdownItem svg {
   margin-left: 1em;
   fill: var(--ps-neutral-2);

diff --git a/...ges/pyroscope-flamegraph/src/FlameGraph/FlameGraphComponent/DiffLegendPaletteDropdown.tsx b/...ges/pyroscope-flamegraph/src/FlameGraph/FlameGraphComponent/DiffLegendPaletteDropdown.tsx
@@ -40,6 +40,7 @@ export const DiffLegendPaletteDropdown = (
       <div ref={legendRef} className={styles.dropdownWrapper}>
         <Dropdown
           label="Select a palette"
+          align="end"
           menuButton={
             <MenuButton
               className={cx(

diff --git a/packages/pyroscope-flamegraph/src/FlameGraph/FlameGraphComponent/index.tsx b/packages/pyroscope-flamegraph/src/FlameGraph/FlameGraphComponent/index.tsx
@@ -1,12 +1,12 @@
-/* eslint-disable no-unused-expressions */
+/* eslint-disable no-unused-expressions, import/no-extraneous-dependencies */
 import React, { useCallback, useRef } from 'react';
 import clsx from 'clsx';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import { faRedo } from '@fortawesome/free-solid-svg-icons/faRedo';
 import { faCopy } from '@fortawesome/free-solid-svg-icons/faCopy';
 import { faHighlighter } from '@fortawesome/free-solid-svg-icons/faHighlighter';
 import { faCompressAlt } from '@fortawesome/free-solid-svg-icons/faCompressAlt';
-import { MenuItem } from '@szhsin/react-menu';
+import { MenuItem } from '@webapp/ui/Menu';
 import useResizeObserver from '@react-hook/resize-observer';
 import { Maybe } from 'true-myth';
 import debounce from 'lodash.debounce';
@@ -235,7 +235,7 @@ export default function FlameGraphComponent(props: FlamegraphProps) {
 
         return (
           <MenuItem
-            key="highlight-similar-nodes"
+            key="open-in-sandwich-view"
             className={indexStyles.sandwichItem}
             onClick={handleClick}
           >

diff --git a/packages/pyroscope-flamegraph/src/__snapshots__/Toolbar.spec.tsx.snap b/packages/pyroscope-flamegraph/src/__snapshots__/Toolbar.spec.tsx.snap
@@ -277,14 +277,11 @@ exports[`ProfileHeader shifts between visualization modes 2`] = `
           aria-expanded="false"
           aria-haspopup="true"
           aria-label="Fit Mode"
-          class="rc-menu-button dropdownMenuButton fitModeDropdownSmall"
+          class="szh-menu-button dropdownMenuButton fitModeDropdownSmall"
           type="button"
         >
           Tail
         </button>
-        <div
-          class="rc-menu-container"
-        />
       </div>
       <div
         class="divider"
@@ -361,14 +358,11 @@ exports[`ProfileHeader shifts between visualization modes 2`] = `
             aria-expanded="false"
             aria-haspopup="true"
             aria-label="View Mode"
-            class="rc-menu-button dropdownMenuButton viewModeDropdownButton"
+            class="szh-menu-button dropdownMenuButton viewModeDropdownButton"
             type="button"
           >
             Table and Flamegraph
           </button>
-          <div
-            class="rc-menu-container"
-          />
         </div>
       </div>
     </div>

diff --git a/webapp/javascript/components/Settings/Users/getUserTableRows.tsx b/webapp/javascript/components/Settings/Users/getUserTableRows.tsx
@@ -1,5 +1,5 @@
 import React, { useState } from 'react';
-import type { ClickEvent } from '@szhsin/react-menu';
+import type { ClickEvent } from '@webapp/ui/Menu';
 import { formatRelative } from 'date-fns';
 import { faTimes } from '@fortawesome/free-solid-svg-icons/faTimes';
 import { faCheck } from '@fortawesome/free-solid-svg-icons/faCheck';

diff --git a/webapp/javascript/pages/TagExplorerView.tsx b/webapp/javascript/pages/TagExplorerView.tsx
@@ -1,7 +1,7 @@
 import React, { useEffect, useMemo } from 'react';
 import { NavLink, useLocation } from 'react-router-dom';
 import type { Maybe } from 'true-myth';
-import type { ClickEvent } from '@szhsin/react-menu';
+import type { ClickEvent } from '@webapp/ui/Menu';
 import Color from 'color';
 import TotalSamplesChart from '@webapp/pages/tagExplorer/components/TotalSamplesChart';
 import type { Profile } from '@pyroscope/models/src';

diff --git a/webapp/javascript/pages/continuous/contextMenu/AddAnnotation.menuitem.tsx b/webapp/javascript/pages/continuous/contextMenu/AddAnnotation.menuitem.tsx
@@ -1,6 +1,6 @@
 /* eslint-disable react/jsx-props-no-spreading */
 import React, { useState } from 'react';
-import { MenuItem, applyStatics } from '@webapp/ui/Menu';
+import { MenuItem } from '@webapp/ui/Menu';
 import {
   Popover,
   PopoverBody,
@@ -102,9 +102,4 @@ function AddAnnotation(props: AddAnnotationProps) {
   );
 }
 
-// TODO: get rid of this in v3
-// https://szhsin.github.io/react-menu-v2/docs#utils-apply-statics
-// https://github.com/pyroscope-io/pyroscope/issues/1525
-applyStatics(MenuItem)(AddAnnotation);
-
 export default AddAnnotation;
diff --git a/webapp/javascript/pages/continuous/contextMenu/AnnotationInfo.tsx b/webapp/javascript/pages/continuous/contextMenu/AnnotationInfo.tsx
@@ -1,6 +1,5 @@
 /* eslint-disable react/jsx-props-no-spreading */
 import React, { Dispatch, SetStateAction } from 'react';
-import { MenuItem, applyStatics } from '@webapp/ui/Menu';
 import { Popover, PopoverBody, PopoverFooter } from '@webapp/ui/Popover';
 import Button from '@webapp/ui/Button';
 import { Portal } from '@webapp/ui/Portal';
@@ -69,6 +68,4 @@ const AnnotationInfo = ({
   );
 };
 
-applyStatics(MenuItem)(AnnotationInfo);
-
 export default AnnotationInfo;
diff --git a/webapp/javascript/pages/continuous/contextMenu/ContextMenu.tsx b/webapp/javascript/pages/continuous/contextMenu/ContextMenu.tsx
@@ -1,5 +1,6 @@
-import React, { useEffect, useState } from 'react';
-import { ControlledMenu } from '@webapp/ui/Menu';
+/* eslint-disable react/jsx-props-no-spreading */
+import React, { useEffect } from 'react';
+import { ControlledMenu, useMenuState } from '@webapp/ui/Menu';
 import { ContextMenuProps as PluginContextMenuProps } from '@webapp/components/TimelineChart/ContextMenu.plugin';
 
 interface ContextMenuProps {
@@ -12,19 +13,19 @@ interface ContextMenuProps {
 
 function ContextMenu(props: ContextMenuProps) {
   const { position, children } = props;
-  const [isOpen, setOpen] = useState(false);
+  const [menuProps, toggleMenu] = useMenuState({ transition: true });
 
   // https://github.com/szhsin/react-menu/issues/2#issuecomment-719166062
   useEffect(() => {
-    setOpen(true);
+    toggleMenu(true);
   }, []);
 
   return (
     <>
       <ControlledMenu
-        isOpen={isOpen}
+        {...menuProps}
         anchorPoint={{ x: position.pageX, y: position.pageY }}
-        onClose={() => setOpen(false)}
+        onClose={() => toggleMenu(false)}
       >
         {children}
       </ControlledMenu>

diff --git a/webapp/javascript/ui/Dropdown.module.scss b/webapp/javascript/ui/Dropdown.module.scss
@@ -6,26 +6,30 @@
   background-color: var(--ps-dropdown-background) !important;
   color: var(--ps-neutral-2) !important;
 
-  & ul[class*='rc-menu'] {
+  & ul[class*='szh-menu'] {
     color: var(--ps-neutral-2) !important;
     background-color: var(--ps-dropdown-background) !important;
   }
 
+  li[class='szh-menu__header'] {
+    text-align: left;
+  }
+
   & li[class*='hover'],
   div[class*='hover'] {
     background-color: var(--ps-ui-element-bg-highlight);
   }
 
-  [class*='rc-menu--open']:empty {
+  [class*='szh-menu--open']:empty {
     display: none;
   }
 
-  [class*='rc-menu__item active'] {
+  [class*='szh-menu__item active'] {
     background-color: var(--ps-ui-element-bg-highlight);
     color: var(--ps-selected-app);
   }
 
-  [class*='rc-menu__item--focusable'] {
+  [class*='szh-menu__item--focusable'] {
     // important to ignore the hover effect
     background-color: inherit !important;
     padding-top: 1rem;

diff --git a/webapp/javascript/ui/Menu.tsx b/webapp/javascript/ui/Menu.tsx
@@ -2,7 +2,8 @@ import {
   Menu,
   MenuItem,
   ControlledMenu,
-  applyStatics,
+  useMenuState,
+  ClickEvent,
 } from '@szhsin/react-menu';
 
-export { ControlledMenu, Menu, MenuItem, applyStatics };
+export { ControlledMenu, Menu, MenuItem, useMenuState, ClickEvent };
diff --git a/yarn.lock b/yarn.lock
@@ -4859,12 +4859,13 @@
     regenerator-runtime "^0.13.7"
     resolve-from "^5.0.0"
 
-"@szhsin/react-menu@^1.10.1":
-  version "1.11.0"
-  resolved "https://registry.yarnpkg.com/@szhsin/react-menu/-/react-menu-1.11.0.tgz#3b4929d5cf5bb676eac3dc962da3029080b82bc2"
-  integrity sha512-kaW/IpcokmyVwX7WGe6ijQI/xQDw5nQXUKyiioy40dm5gt45+Lo7cqGVUoy7Qj8275Q9tgN6hcazWX/DCcYUfQ==
+"@szhsin/react-menu@3.3.0":
+  version "3.3.0"
+  resolved "https://registry.yarnpkg.com/@szhsin/react-menu/-/react-menu-3.3.0.tgz#66b921b31938a665b665f80e24d9ca2800360c45"
+  integrity sha512-GZfPLey2ZSVDaRtALuqH611cSM8QBujztYqLDqrqbWkTCwXrdob9G8PaBapBR/5hyW6dqDKJzFfcthZE3/iMpg==
   dependencies:
     prop-types "^15.7.2"
+    react-transition-state "^1.1.5"
 
 "@szmarczak/http-timer@^4.0.0":
   version "4.0.6"
@@ -19020,6 +19021,11 @@ react-transition-group@^4.4.5:
     loose-envify "^1.4.0"
     prop-types "^15.6.2"
 
+react-transition-state@^1.1.5:
+  version "1.1.5"
+  resolved "https://registry.yarnpkg.com/react-transition-state/-/react-transition-state-1.1.5.tgz#22accee21d0011b1d0245be24b6262ae67f494c3"
+  integrity sha512-ITY2mZqc2dWG2eitJkYNdcSFW8aKeOlkL2A/vowRrLL8GH3J6Re/SpD/BLvQzrVOTqjsP0b5S9N10vgNNzwMUQ==
+
 react-universal-interface@^0.6.2:
   version "0.6.2"
   resolved "https://registry.yarnpkg.com/react-universal-interface/-/react-universal-interface-0.6.2.tgz#5e8d438a01729a4dbbcbeeceb0b86be146fe2b3b"