From 615f6973570a8c6385b662bdc17ee41342d413f0 Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Mon, 21 Feb 2022 15:18:55 -0500 Subject: [PATCH 1/3] Try harder to keep context menus inside the window Signed-off-by: Robin Townsend --- src/components/structures/ContextMenu.tsx | 54 +++++++++++++++++------ 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index fa164bca17a..8e5f0565cf8 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,48 @@ 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 + if (contextMenuRect) { + if (position.top !== undefined) { + position.top = Math.min( + position.top, + document.body.clientHeight - 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, + document.body.clientHeight - 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, + document.body.clientWidth - 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, + document.body.clientWidth - 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; From 3dfd88319773eedbbd9379e4590a4371cce81405 Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Wed, 23 Feb 2022 11:04:19 -0500 Subject: [PATCH 2/3] Use UIStore for window dimensions Signed-off-by: Robin Townsend --- src/components/structures/ContextMenu.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index 8e5f0565cf8..bbdcae2eebc 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -254,11 +254,12 @@ export default class ContextMenu extends React.PureComponent { // 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, - document.body.clientHeight - contextMenuRect.height - WINDOW_PADDING, + windowHeight - contextMenuRect.height - WINDOW_PADDING, ); // Adjust the chevron if necessary if (chevronOffset.top !== undefined) { @@ -267,7 +268,7 @@ export default class ContextMenu extends React.PureComponent { } else if (position.bottom !== undefined) { position.bottom = Math.min( position.bottom, - document.body.clientHeight - contextMenuRect.height - WINDOW_PADDING, + windowHeight - contextMenuRect.height - WINDOW_PADDING, ); if (chevronOffset.top !== undefined) { chevronOffset.top = props.chevronOffset + props.bottom - position.bottom; @@ -276,7 +277,7 @@ export default class ContextMenu extends React.PureComponent { if (position.left !== undefined) { position.left = Math.min( position.left, - document.body.clientWidth - contextMenuRect.width - WINDOW_PADDING, + windowWidth - contextMenuRect.width - WINDOW_PADDING, ); if (chevronOffset.left !== undefined) { chevronOffset.left = props.chevronOffset + props.left - position.left; @@ -284,7 +285,7 @@ export default class ContextMenu extends React.PureComponent { } else if (position.right !== undefined) { position.right = Math.min( position.right, - document.body.clientWidth - contextMenuRect.width - WINDOW_PADDING, + windowWidth - contextMenuRect.width - WINDOW_PADDING, ); if (chevronOffset.left !== undefined) { chevronOffset.left = props.chevronOffset + props.right - position.right; From c2b552b2fb1f7edd1413acd4335c6e5c0b48e39c Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Wed, 23 Feb 2022 17:57:54 -0500 Subject: [PATCH 3/3] Test ContextMenu positioning Signed-off-by: Robin Townsend --- .../views/context_menus/ContextMenu-test.tsx | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 test/components/views/context_menus/ContextMenu-test.tsx 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); + }); +});