Fix edge case in context menu chevron positioning (#7899)

* Fix edge case in context menu chevron positioning Signed-off-by: Robin Townsend <robin@robin.town> * Expand context menu positioning regression tests Signed-off-by: Robin Townsend <robin@robin.town>
matrix-org · Feb 25, 2022 · ece2b85 · ece2b85
1 parent 3e4e7ef
commit ece2b85
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 20 deletions.
diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx
@@ -271,7 +271,7 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
                     windowHeight - contextMenuRect.height - WINDOW_PADDING,
                 );
                 if (chevronOffset.top !== undefined) {
-                    chevronOffset.top = props.chevronOffset + props.bottom - position.bottom;
+                    chevronOffset.top = props.chevronOffset + position.bottom - props.bottom;
                 }
             }
             if (position.left !== undefined) {
@@ -288,7 +288,7 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
                     windowWidth - contextMenuRect.width - WINDOW_PADDING,
                 );
                 if (chevronOffset.left !== undefined) {
-                    chevronOffset.left = props.chevronOffset + props.right - position.right;
+                    chevronOffset.left = props.chevronOffset + position.right - props.right;
                 }
             }
         }

diff --git a/test/components/views/context_menus/ContextMenu-test.tsx b/test/components/views/context_menus/ContextMenu-test.tsx
@@ -34,26 +34,101 @@ describe("<ContextMenu />", () => {
         height: menuSize,
     });
 
-    const targetY = windowSize - menuSize + 50;
     const targetChevronOffset = 25;
 
-    const wrapper = mount(
-        <ContextMenu
-            top={targetY}
-            left={0}
-            chevronFace={ChevronFace.Right}
-            chevronOffset={targetChevronOffset}
-        />,
-    );
-    const chevron = wrapper.find(".mx_ContextualMenu_chevron_right");
-
-    const actualY = parseInt(wrapper.getDOMNode().style.getPropertyValue("top"));
-    const actualChevronOffset = parseInt(chevron.getDOMNode().style.getPropertyValue("top"));
-
-    it("stays within the window", () => {
-        expect(actualY + menuSize).toBeLessThanOrEqual(windowSize);
+    describe("near top edge of window", () => {
+        const targetY = -50;
+
+        const wrapper = mount(
+            <ContextMenu
+                bottom={windowSize - targetY - menuSize}
+                right={menuSize}
+                chevronFace={ChevronFace.Left}
+                chevronOffset={targetChevronOffset}
+            />,
+        );
+        const chevron = wrapper.find(".mx_ContextualMenu_chevron_left");
+
+        const actualY = windowSize - parseInt(wrapper.getDOMNode().style.getPropertyValue("bottom")) - menuSize;
+        const actualChevronOffset = parseInt(chevron.getDOMNode().style.getPropertyValue("top"));
+
+        it("stays within the window", () => {
+            expect(actualY).toBeGreaterThanOrEqual(0);
+        });
+        it("positions the chevron correctly", () => {
+            expect(actualChevronOffset).toEqual(targetChevronOffset + targetY - actualY);
+        });
     });
-    it("positions the chevron correctly", () => {
-        expect(actualChevronOffset).toEqual(targetChevronOffset + targetY - actualY);
+
+    describe("near right edge of window", () => {
+        const targetX = windowSize - menuSize + 50;
+
+        const wrapper = mount(
+            <ContextMenu
+                bottom={0}
+                left={targetX}
+                chevronFace={ChevronFace.Top}
+                chevronOffset={targetChevronOffset}
+            />,
+        );
+        const chevron = wrapper.find(".mx_ContextualMenu_chevron_top");
+
+        const actualX = parseInt(wrapper.getDOMNode().style.getPropertyValue("left"));
+        const actualChevronOffset = parseInt(chevron.getDOMNode().style.getPropertyValue("left"));
+
+        it("stays within the window", () => {
+            expect(actualX + menuSize).toBeLessThanOrEqual(windowSize);
+        });
+        it("positions the chevron correctly", () => {
+            expect(actualChevronOffset).toEqual(targetChevronOffset + targetX - actualX);
+        });
+    });
+
+    describe("near bottom edge of window", () => {
+        const targetY = windowSize - menuSize + 50;
+
+        const wrapper = mount(
+            <ContextMenu
+                top={targetY}
+                left={0}
+                chevronFace={ChevronFace.Right}
+                chevronOffset={targetChevronOffset}
+            />,
+        );
+        const chevron = wrapper.find(".mx_ContextualMenu_chevron_right");
+
+        const actualY = parseInt(wrapper.getDOMNode().style.getPropertyValue("top"));
+        const actualChevronOffset = parseInt(chevron.getDOMNode().style.getPropertyValue("top"));
+
+        it("stays within the window", () => {
+            expect(actualY + menuSize).toBeLessThanOrEqual(windowSize);
+        });
+        it("positions the chevron correctly", () => {
+            expect(actualChevronOffset).toEqual(targetChevronOffset + targetY - actualY);
+        });
+    });
+
+    describe("near left edge of window", () => {
+        const targetX = -50;
+
+        const wrapper = mount(
+            <ContextMenu
+                top={0}
+                right={windowSize - targetX - menuSize}
+                chevronFace={ChevronFace.Bottom}
+                chevronOffset={targetChevronOffset}
+            />,
+        );
+        const chevron = wrapper.find(".mx_ContextualMenu_chevron_bottom");
+
+        const actualX = windowSize - parseInt(wrapper.getDOMNode().style.getPropertyValue("right")) - menuSize;
+        const actualChevronOffset = parseInt(chevron.getDOMNode().style.getPropertyValue("left"));
+
+        it("stays within the window", () => {
+            expect(actualX).toBeGreaterThanOrEqual(0);
+        });
+        it("positions the chevron correctly", () => {
+            expect(actualChevronOffset).toEqual(targetChevronOffset + targetX - actualX);
+        });
     });
 });