From eb789a0ab15cffa83c6585011ddf27aa79ad8c60 Mon Sep 17 00:00:00 2001
From: Juho Heikka <juho.heikka@gmail.com>
Date: Tue, 21 Sep 2021 23:53:11 +0300
Subject: [PATCH] Fix menu rendering on when slightly off screen

Menu would bounce around in some cases because the
refreshPosition() would calculate different results
based on different state of the component and previous
render cycle CSS classes.

Also I found that the render function calls causes
in an infinite loop while open because of refreshPosition
will cause a state change which will rerender the function.
Fixed that with the shouldComponentUpdate which checks
if state has actually changed in between renders.

Signed-off-by: Juho Heikka <juho.heikka@gmail.com>
---
 src/renderer/components/menu/menu.scss |  8 ----
 src/renderer/components/menu/menu.tsx  | 54 +++++++++++++++-----------
 2 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/src/renderer/components/menu/menu.scss b/src/renderer/components/menu/menu.scss
index d94cbff63e..0db62e278a 100644
--- a/src/renderer/components/menu/menu.scss
+++ b/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) {
diff --git a/src/renderer/components/menu/menu.tsx b/src/renderer/components/menu/menu.tsx
index 30780e7ac4..8ceef6bf8e 100644
--- a/src/renderer/components/menu/menu.tsx
+++ b/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;
@@ -85,6 +86,10 @@ export class Menu extends React.Component<MenuProps, State> {
     return !!this.props.isOpen;
   }
 
+  shouldComponentUpdate(nextProps: MenuProps, nextState: State) {
+    return !isEqual(nextState, this.state) || !isEqual(nextProps, this.props);
+  }
+
   componentDidMount() {
     if (!this.props.usePortal) {
       const parent = this.elem.parentElement;
@@ -150,39 +155,42 @@ export class Menu extends React.Component<MenuProps, State> {
       return;
     }
 
-    const { width, height } = this.opener.getBoundingClientRect();
-    let { left, top, bottom, right } = this.opener.getBoundingClientRect();
+    const openerClientRect = 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;
-      top += window.pageYOffset;
-      right = left + width;
-      bottom = top + height;
+      openerLeft += window.pageXOffset;
+      openerTop += window.pageYOffset;
+      openerRight = openerLeft + openerClientRect.width;
+      openerBottom = openerTop + openerClientRect.height;
     }
 
-    // setup initial position
-    const position: MenuPosition = { left: true, bottom: true };
+    const extraMargin = this.props.usePortal ? 8 : 0;
 
-    this.elem.style.left = `${left}px`;
-    this.elem.style.top = `${bottom}px`;
+    const { width: menuWidth, height: menuHeight } = this.elem.getBoundingClientRect();
 
-    // correct position if menu doesn't fit to viewport
-    const menuPos = this.elem.getBoundingClientRect();
+    const rightSideOfMenu = openerLeft + menuWidth;
+    const renderMenuLeft = rightSideOfMenu > window.innerWidth;
+    const menuOnLeftSidePosition = `${openerRight - this.elem.offsetWidth}px`;
+    const menuOnRightSidePosition = `${openerLeft}px`;
 
-    if (menuPos.right > window.innerWidth) {
-      this.elem.style.left = `${right - this.elem.offsetWidth}px`;
-      position.right = true;
-      delete position.left;
-    }
+    this.elem.style.left = renderMenuLeft ? menuOnLeftSidePosition : menuOnRightSidePosition;
 
-    if (menuPos.bottom > window.innerHeight) {
-      this.elem.style.top = `${top - this.elem.offsetHeight}px`;
-      position.top = true;
-      delete position.bottom;
-    }
-    this.setState({ position });
+    const bottomOfMenu = openerBottom + extraMargin + menuHeight;
+    const renderMenuOnTop = bottomOfMenu > window.innerHeight;
+    const menuOnTopPosition = `${openerTop - this.elem.offsetHeight - extraMargin}px`;
+    const menuOnBottomPosition = `${openerBottom + extraMargin}px`;
+
+    this.elem.style.top = renderMenuOnTop ? menuOnTopPosition : menuOnBottomPosition;
+
+    this.setState({ position: {
+      top: renderMenuOnTop,
+      bottom: !renderMenuOnTop,
+      left: renderMenuLeft,
+      right: !renderMenuLeft
+    } });
   };
 
   open() {