Merge pull request #3866 from lensapp/issue-3383-menu-position-bug

Fix menu rendering on when slightly off screen

src/renderer/components/menu/menu.scss

@@ -35,14 +35,6 @@
   &.portal {
     left: -1000px;
     top: -1000px;
-
-    &.top {
-      margin-top: -$margin;
-    }
-
-    &.bottom {
-      margin-top: $margin;
-    }
   }
 
   &:not(.portal) {

src/renderer/components/menu/menu.tsx

@@ -26,6 +26,7 @@ import { createPortal } from "react-dom";
 import { autoBind, cssNames, noop } from "../../utils";
 import { Animate } from "../animate";
 import { Icon, IconProps } from "../icon";
+import isEqual from "lodash/isEqual";
 
 export const MenuContext = React.createContext<MenuContextValue>(null);
 export type MenuContextValue = Menu;
@@ -37,6 +38,10 @@ export interface MenuPosition {
   bottom?: boolean;
 }
 
+export interface MenuStyle {
+  top: string;
+  left: string;
+}
 export interface MenuProps {
   isOpen?: boolean;
   open(): void;
@@ -56,6 +61,7 @@ export interface MenuProps {
 
 interface State {
   position?: MenuPosition;
+  menuStyle?: MenuStyle
 }
 
 const defaultPropsMenu: Partial<MenuProps> = {
@@ -75,7 +81,6 @@ export class Menu extends React.Component<MenuProps, State> {
     super(props);
     autoBind(this);
   }
-
   public opener: HTMLElement;
   public elem: HTMLUListElement;
   protected items: { [index: number]: MenuItem } = {};
@@ -119,6 +124,12 @@ export class Menu extends React.Component<MenuProps, State> {
     window.removeEventListener("scroll", this.onScrollOutside, true);
   }
 
+  componentDidUpdate(prevProps: MenuProps) {
+    if (!isEqual(prevProps.children, this.props.children)) {
+      this.refreshPosition();
+    }
+  }
+
   protected get focusableItems() {
     return Object.values(this.items).filter(item => item.isFocusable);
   }
@@ -150,39 +161,44 @@ export class Menu extends React.Component<MenuProps, State> {
       return;
     }
 
-    const { width, height } = this.opener.getBoundingClientRect();
+    const openerClientRect = this.opener.getBoundingClientRect();
-    let { left, top, bottom, right } = this.opener.getBoundingClientRect();
+    let { left: openerLeft, top: openerTop, bottom: openerBottom, right: openerRight } = this.opener.getBoundingClientRect();
     const withScroll = window.getComputedStyle(this.elem).position !== "fixed";
 
     // window global scroll corrections
     if (withScroll) {
-      left += window.pageXOffset;
+      openerLeft += window.pageXOffset;
-      top += window.pageYOffset;
+      openerTop += window.pageYOffset;
-      right = left + width;
+      openerRight = openerLeft + openerClientRect.width;
-      bottom = top + height;
+      openerBottom = openerTop + openerClientRect.height;
     }
 
-    // setup initial position
+    const extraMargin = this.props.usePortal ? 8 : 0;
-    const position: MenuPosition = { left: true, bottom: true };
 
-    this.elem.style.left = `${left}px`;
+    const { width: menuWidth, height: menuHeight } = this.elem.getBoundingClientRect();
-    this.elem.style.top = `${bottom}px`;
 
-    // correct position if menu doesn't fit to viewport
+    const rightSideOfMenu = openerLeft + menuWidth;
-    const menuPos = this.elem.getBoundingClientRect();
+    const renderMenuLeft = rightSideOfMenu > window.innerWidth;
+    const menuOnLeftSidePosition = `${openerRight - this.elem.offsetWidth}px`;
+    const menuOnRightSidePosition = `${openerLeft}px`;
 
-    if (menuPos.right > window.innerWidth) {
+    const bottomOfMenu = openerBottom + extraMargin + menuHeight;
-      this.elem.style.left = `${right - this.elem.offsetWidth}px`;
+    const renderMenuOnTop = bottomOfMenu > window.innerHeight;
-      position.right = true;
+    const menuOnTopPosition = `${openerTop - this.elem.offsetHeight - extraMargin}px`;
-      delete position.left;
+    const menuOnBottomPosition = `${openerBottom + extraMargin}px`;
-    }
 
-    if (menuPos.bottom > window.innerHeight) {
+    this.setState({
-      this.elem.style.top = `${top - this.elem.offsetHeight}px`;
+      position: {
-      position.top = true;
+        top: renderMenuOnTop,
-      delete position.bottom;
+        bottom: !renderMenuOnTop,
-    }
+        left: renderMenuLeft,
-    this.setState({ position });
+        right: !renderMenuLeft
+      },
+      menuStyle: {
+        top: renderMenuOnTop ? menuOnTopPosition : menuOnBottomPosition,
+        left: renderMenuLeft ? menuOnLeftSidePosition : menuOnRightSidePosition,
+      }
+    });
   };
 
   open() {
@@ -276,10 +292,6 @@ export class Menu extends React.Component<MenuProps, State> {
   }
 
   render() {
-    if (this.isOpen) {
-      setImmediate(() => this.refreshPosition());
-    }
-
     const { position, id } = this.props;
     let { className, usePortal } = this.props;
 
@@ -305,7 +317,15 @@ export class Menu extends React.Component<MenuProps, State> {
     const menu = (
       <MenuContext.Provider value={this}>
         <Animate enter={this.isOpen}>
-          <ul id={id} className={className} ref={this.bindRef}>
+          <ul
+            id={id}
+            ref={this.bindRef}
+            className={className}
+            style={{
+              left: this.state?.menuStyle?.left,
+              top: this.state?.menuStyle?.top
+            }}
+          >
             {menuItems}
           </ul>
         </Animate>