From e913f03a6773c320d409fae6ac09901632748acc Mon Sep 17 00:00:00 2001 From: Dariusz Niemczyk Date: Mon, 13 Sep 2021 22:11:43 +0200 Subject: [PATCH 1/3] Add missing types --- src/components/structures/ContextMenu.tsx | 14 ++++++++++++-- src/components/views/rooms/MessageComposer.tsx | 3 ++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index 332b6cd318..d503e56a6d 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -404,17 +404,27 @@ export class ContextMenu extends React.PureComponent { } } +export type ToRightOf = { + left: number; + top: number; + chevronOffset: number; +}; + // Placement method for to position context menu to right of elementRect with chevronOffset -export const toRightOf = (elementRect: Pick, chevronOffset = 12) => { +export const toRightOf = (elementRect: Pick, chevronOffset = 12): ToRightOf => { const left = elementRect.right + window.pageXOffset + 3; let top = elementRect.top + (elementRect.height / 2) + window.pageYOffset; top -= chevronOffset + 8; // where 8 is half the height of the chevron return { left, top, chevronOffset }; }; +export type AboveLeftOf = IPosition & { + chevronFace: ChevronFace; +}; + // Placement method for to position context menu right-aligned and flowing to the left of elementRect, // and either above or below: wherever there is more space (maybe this should be aboveOrBelowLeftOf?) -export const aboveLeftOf = (elementRect: DOMRect, chevronFace = ChevronFace.None, vPadding = 0) => { +export const aboveLeftOf = (elementRect: DOMRect, chevronFace = ChevronFace.None, vPadding = 0): AboveLeftOf => { const menuOptions: IPosition & { chevronFace: ChevronFace } = { chevronFace }; const buttonRight = elementRect.right + window.pageXOffset; diff --git a/src/components/views/rooms/MessageComposer.tsx b/src/components/views/rooms/MessageComposer.tsx index dd6ce10825..506bf09a92 100644 --- a/src/components/views/rooms/MessageComposer.tsx +++ b/src/components/views/rooms/MessageComposer.tsx @@ -32,6 +32,7 @@ import { ContextMenu, useContextMenu, MenuItem, + AboveLeftOf, } from "../../structures/ContextMenu"; import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; import ReplyPreview from "./ReplyPreview"; @@ -511,7 +512,7 @@ export default class MessageComposer extends React.Component { null, ]; - let menuPosition; + let menuPosition: AboveLeftOf | undefined; if (this.ref.current) { const contentRect = this.ref.current.getBoundingClientRect(); menuPosition = aboveLeftOf(contentRect); From ceafa833929d0d1a3474b65a167766fcfa349b83 Mon Sep 17 00:00:00 2001 From: Dariusz Niemczyk Date: Mon, 13 Sep 2021 22:11:58 +0200 Subject: [PATCH 2/3] Fix invalid ContextualMenu positions --- 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 d503e56a6d..0c2c0e207f 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -322,10 +322,11 @@ export class ContextMenu extends React.PureComponent { const menuClasses = classNames({ 'mx_ContextualMenu': true, - 'mx_ContextualMenu_left': !hasChevron && position.left, - 'mx_ContextualMenu_right': !hasChevron && position.right, - 'mx_ContextualMenu_top': !hasChevron && position.top, - 'mx_ContextualMenu_bottom': !hasChevron && position.bottom, + // Defensively check for counter cases + 'mx_ContextualMenu_left': !hasChevron && position.left !== undefined && !position.right, + 'mx_ContextualMenu_right': !hasChevron && position.right !== undefined && !position.left, + 'mx_ContextualMenu_top': !hasChevron && position.top !== undefined && !position.bottom, + 'mx_ContextualMenu_bottom': !hasChevron && position.bottom !== undefined && !position.top, 'mx_ContextualMenu_withChevron_left': chevronFace === ChevronFace.Left, 'mx_ContextualMenu_withChevron_right': chevronFace === ChevronFace.Right, 'mx_ContextualMenu_withChevron_top': chevronFace === ChevronFace.Top, From 60174c9836aa1c6a7da2b1a8625790ddbd7d5bfb Mon Sep 17 00:00:00 2001 From: Dariusz Niemczyk Date: Tue, 14 Sep 2021 14:51:17 +0200 Subject: [PATCH 3/3] Add a better comment describing the behavior --- src/components/structures/ContextMenu.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index 0c2c0e207f..d65f8e3a10 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -322,7 +322,12 @@ export class ContextMenu extends React.PureComponent { const menuClasses = classNames({ 'mx_ContextualMenu': true, - // Defensively check for counter cases + /** + * In some cases we may get the number of 0, which still means that we're supposed to properly + * add the specific position class, but as it was falsy things didn't work as intended. + * In addition, defensively check for counter cases where we may get more than one value, + * even if we shouldn't. + */ 'mx_ContextualMenu_left': !hasChevron && position.left !== undefined && !position.right, 'mx_ContextualMenu_right': !hasChevron && position.right !== undefined && !position.left, 'mx_ContextualMenu_top': !hasChevron && position.top !== undefined && !position.bottom,