From 85dc58a5043e09c61a46c020402505e12220011a Mon Sep 17 00:00:00 2001 From: Robin Date: Thu, 24 Feb 2022 10:25:27 -0500 Subject: [PATCH] Try harder to keep context menus inside the window (#7863) * Try harder to keep context menus inside the window Signed-off-by: Robin Townsend * Use UIStore for window dimensions Signed-off-by: Robin Townsend * Test ContextMenu positioning Signed-off-by: Robin Townsend --- src/components/structures/ContextMenu.tsx | 55 +++++++++++++---- .../views/context_menus/ContextMenu-test.tsx | 59 +++++++++++++++++++ 2 files changed, 101 insertions(+), 13 deletions(-) create mode 100644 test/components/views/context_menus/ContextMenu-test.tsx diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index fa164bca17a..bbdcae2eebc 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -31,6 +31,7 @@ import { checkInputableElement, RovingTabIndexProvider } from "../../accessibili // of doing reusable widgets like dialog boxes & menus where we go and // pass in a custom control as the actual body. +const WINDOW_PADDING = 10; const ContextualMenuContainerId = "mx_ContextualMenu_Container"; function getOrCreateContainer(): HTMLDivElement { @@ -247,21 +248,49 @@ export default class ContextMenu extends React.PureComponent { if (chevronFace === ChevronFace.Top || chevronFace === ChevronFace.Bottom) { chevronOffset.left = props.chevronOffset; - } else if (position.top !== undefined) { - const target = position.top; - - // By default, no adjustment is made - let adjusted = target; + } else { + chevronOffset.top = props.chevronOffset; + } - // If we know the dimensions of the context menu, adjust its position - // such that it does not leave the (padded) window. - if (contextMenuRect) { - const padding = 10; - adjusted = Math.min(position.top, document.body.clientHeight - contextMenuRect.height - padding); + // If we know the dimensions of the context menu, adjust its position to + // keep it within the bounds of the (padded) window + const { windowWidth, windowHeight } = UIStore.instance; + if (contextMenuRect) { + if (position.top !== undefined) { + position.top = Math.min( + position.top, + windowHeight - contextMenuRect.height - WINDOW_PADDING, + ); + // Adjust the chevron if necessary + if (chevronOffset.top !== undefined) { + chevronOffset.top = props.chevronOffset + props.top - position.top; + } + } else if (position.bottom !== undefined) { + position.bottom = Math.min( + position.bottom, + windowHeight - contextMenuRect.height - WINDOW_PADDING, + ); + if (chevronOffset.top !== undefined) { + chevronOffset.top = props.chevronOffset + props.bottom - position.bottom; + } + } + if (position.left !== undefined) { + position.left = Math.min( + position.left, + windowWidth - contextMenuRect.width - WINDOW_PADDING, + ); + if (chevronOffset.left !== undefined) { + chevronOffset.left = props.chevronOffset + props.left - position.left; + } + } else if (position.right !== undefined) { + position.right = Math.min( + position.right, + windowWidth - contextMenuRect.width - WINDOW_PADDING, + ); + if (chevronOffset.left !== undefined) { + chevronOffset.left = props.chevronOffset + props.right - position.right; + } } - - position.top = adjusted; - chevronOffset.top = Math.max(props.chevronOffset, props.chevronOffset + target - adjusted); } let chevron; diff --git a/test/components/views/context_menus/ContextMenu-test.tsx b/test/components/views/context_menus/ContextMenu-test.tsx new file mode 100644 index 00000000000..0b34b7dfd46 --- /dev/null +++ b/test/components/views/context_menus/ContextMenu-test.tsx @@ -0,0 +1,59 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React from "react"; +import { mount } from "enzyme"; + +import "../../../skinned-sdk"; +import ContextMenu, { ChevronFace } from "../../../../src/components/structures/ContextMenu.tsx"; +import UIStore from "../../../../src/stores/UIStore.ts"; + +describe("", () => { + // Hardcode window and menu dimensions + const windowSize = 300; + const menuSize = 200; + jest.spyOn(UIStore, "instance", "get").mockImplementation(() => ({ + windowWidth: windowSize, + windowHeight: windowSize, + })); + window.Element.prototype.getBoundingClientRect = jest.fn().mockReturnValue({ + width: menuSize, + height: menuSize, + }); + + const targetY = windowSize - menuSize + 50; + const targetChevronOffset = 25; + + const wrapper = mount( + , + ); + 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); + }); +});