fix: popover positioning (#3399)

src/components/App.tsx

@@ -474,6 +474,7 @@ class App extends React.Component<AppProps, AppState> {
           UIOptions={this.props.UIOptions}
         />
         <div className="excalidraw-textEditorContainer" />
+        <div className="excalidraw-contextMenuContainer" />
         {this.state.showStats && (
           <Stats
             appState={this.state}
@@ -3675,12 +3676,20 @@ class App extends React.Component<AppProps, AppState> {
 
     const type = element ? "element" : "canvas";
 
+    const container = this.excalidrawContainerRef.current!;
+    const {
+      top: offsetTop,
+      left: offsetLeft,
+    } = container.getBoundingClientRect();
+    const left = event.clientX - offsetLeft;
+    const top = event.clientY - offsetTop;
+
     if (element && !this.state.selectedElementIds[element.id]) {
       this.setState({ selectedElementIds: { [element.id]: true } }, () => {
-        this._openContextMenu(event, type);
+        this._openContextMenu({ top, left }, type);
       });
     } else {
-      this._openContextMenu(event, type);
+      this._openContextMenu({ top, left }, type);
     }
   };
 
@@ -3774,11 +3783,11 @@ class App extends React.Component<AppProps, AppState> {
   /** @private use this.handleCanvasContextMenu */
   private _openContextMenu = (
     {
-      clientX,
-      clientY,
+      left,
+      top,
     }: {
-      clientX: number;
-      clientY: number;
+      left: number;
+      top: number;
     },
     type: "canvas" | "element",
   ) => {
@@ -3829,10 +3838,11 @@ class App extends React.Component<AppProps, AppState> {
 
       ContextMenu.push({
         options: viewModeOptions,
-        top: clientY,
-        left: clientX,
+        top,
+        left,
         actionManager: this.actionManager,
         appState: this.state,
+        container: this.excalidrawContainerRef.current!,
       });
 
       if (this.state.viewModeEnabled) {
@@ -3872,10 +3882,11 @@ class App extends React.Component<AppProps, AppState> {
             actionToggleViewMode,
           actionToggleStats,
         ],
-        top: clientY,
-        left: clientX,
+        top,
+        left,
         actionManager: this.actionManager,
         appState: this.state,
+        container: this.excalidrawContainerRef.current!,
       });
       return;
     }
@@ -3883,10 +3894,11 @@ class App extends React.Component<AppProps, AppState> {
     if (this.state.viewModeEnabled) {
       ContextMenu.push({
         options: [navigator.clipboard && actionCopy, ...options],
-        top: clientY,
-        left: clientX,
+        top,
+        left,
         actionManager: this.actionManager,
         appState: this.state,
+        container: this.excalidrawContainerRef.current!,
       });
       return;
     }
@@ -3928,10 +3940,11 @@ class App extends React.Component<AppProps, AppState> {
         actionDuplicateSelection,
         actionDeleteSelected,
       ],
-      top: clientY,
-      left: clientX,
+      top,
+      left,
       actionManager: this.actionManager,
       appState: this.state,
+      container: this.excalidrawContainerRef.current!,
     });
   };
 

src/components/ContextMenu.tsx

@@ -32,67 +32,63 @@ const ContextMenu = ({
   actionManager,
   appState,
 }: ContextMenuProps) => {
-  const isDarkTheme = !!document
-    .querySelector(".excalidraw")
-    ?.classList.contains("theme--dark");
   return (
-    <div
-      className={clsx("excalidraw", {
-        "theme--dark theme--dark-background-none": isDarkTheme,
-      })}
+    <Popover
+      onCloseRequest={onCloseRequest}
+      top={top}
+      left={left}
+      fitInViewport={true}
     >
-      <Popover
-        onCloseRequest={onCloseRequest}
-        top={top}
-        left={left}
-        fitInViewport={true}
+      <ul
+        className="context-menu"
+        onContextMenu={(event) => event.preventDefault()}
       >
-        <ul
-          className="context-menu"
-          onContextMenu={(event) => event.preventDefault()}
-        >
-          {options.map((option, idx) => {
-            if (option === "separator") {
-              return <hr key={idx} className="context-menu-option-separator" />;
-            }
+        {options.map((option, idx) => {
+          if (option === "separator") {
+            return <hr key={idx} className="context-menu-option-separator" />;
+          }
 
-            const actionName = option.name;
-            const label = option.contextItemLabel
-              ? t(option.contextItemLabel)
-              : "";
-            return (
-              <li key={idx} data-testid={actionName} onClick={onCloseRequest}>
-                <button
-                  className={clsx("context-menu-option", {
-                    dangerous: actionName === "deleteSelectedElements",
-                    checkmark: option.checked?.(appState),
-                  })}
-                  onClick={() => actionManager.executeAction(option)}
-                >
-                  <div className="context-menu-option__label">{label}</div>
-                  <kbd className="context-menu-option__shortcut">
-                    {actionName
-                      ? getShortcutFromShortcutName(actionName as ShortcutName)
-                      : ""}
-                  </kbd>
-                </button>
-              </li>
-            );
-          })}
-        </ul>
-      </Popover>
-    </div>
+          const actionName = option.name;
+          const label = option.contextItemLabel
+            ? t(option.contextItemLabel)
+            : "";
+          return (
+            <li key={idx} data-testid={actionName} onClick={onCloseRequest}>
+              <button
+                className={clsx("context-menu-option", {
+                  dangerous: actionName === "deleteSelectedElements",
+                  checkmark: option.checked?.(appState),
+                })}
+                onClick={() => actionManager.executeAction(option)}
+              >
+                <div className="context-menu-option__label">{label}</div>
+                <kbd className="context-menu-option__shortcut">
+                  {actionName
+                    ? getShortcutFromShortcutName(actionName as ShortcutName)
+                    : ""}
+                </kbd>
+              </button>
+            </li>
+          );
+        })}
+      </ul>
+    </Popover>
   );
 };
 
-let contextMenuNode: HTMLDivElement;
-const getContextMenuNode = (): HTMLDivElement => {
+const contextMenuNodeByContainer = new WeakMap<HTMLElement, HTMLDivElement>();
+
+const getContextMenuNode = (container: HTMLElement): HTMLDivElement => {
+  let contextMenuNode = contextMenuNodeByContainer.get(container);
   if (contextMenuNode) {
     return contextMenuNode;
   }
-  const div = document.createElement("div");
-  document.body.appendChild(div);
-  return (contextMenuNode = div);
+  contextMenuNode = document.createElement("div");
+  container
+    .querySelector(".excalidraw-contextMenuContainer")!
+    .appendChild(contextMenuNode);
+  contextMenuNodeByContainer.set(container, contextMenuNode);
+  return contextMenuNode;
 };
 
 type ContextMenuParams = {
@@ -101,10 +97,16 @@ type ContextMenuParams = {
   left: ContextMenuProps["left"];
   actionManager: ContextMenuProps["actionManager"];
   appState: Readonly<AppState>;
+  container: HTMLElement;
 };
 
-const handleClose = () => {
-  unmountComponentAtNode(getContextMenuNode());
+const handleClose = (container: HTMLElement) => {
+  const contextMenuNode = contextMenuNodeByContainer.get(container);
+  if (contextMenuNode) {
+    unmountComponentAtNode(contextMenuNode);
+    contextMenuNode.remove();
+    contextMenuNodeByContainer.delete(container);
+  }
 };
 
 export default {
@@ -121,11 +123,11 @@ export default {
           top={params.top}
           left={params.left}
           options={options}
-          onCloseRequest={handleClose}
+          onCloseRequest={() => handleClose(params.container)}
           actionManager={params.actionManager}
           appState={params.appState}
         />,
-        getContextMenuNode(),
+        getContextMenuNode(params.container),
       );
     }
   },

src/components/Popover.scss

@@ -1,6 +1,6 @@
 .excalidraw {
   .popover {
-    position: fixed;
+    position: absolute;
     z-index: 10;
   }
 }