Fix context menu being opened when clicking message action bar buttons (#9200)

SimonBrandner · web-flow · commit 3eecd6817531 · 2022-08-18T09:18:18.000+02:00
diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx
@@ -225,35 +225,57 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
 
     protected renderMenu(hasBackground = this.props.hasBackground) {
         const position: Partial<Writeable<DOMRect>> = {};
-        const props = this.props;
-
-        if (props.top) {
-            position.top = props.top;
+        const {
+            top,
+            bottom,
+            left,
+            right,
+            bottomAligned,
+            rightAligned,
+            menuClassName,
+            menuHeight,
+            menuWidth,
+            menuPaddingLeft,
+            menuPaddingRight,
+            menuPaddingBottom,
+            menuPaddingTop,
+            zIndex,
+            children,
+            focusLock,
+            managed,
+            wrapperClassName,
+            chevronFace: propsChevronFace,
+            chevronOffset: propsChevronOffset,
+            ...props
+        } = this.props;
+
+        if (top) {
+            position.top = top;
         } else {
-            position.bottom = props.bottom;
+            position.bottom = bottom;
         }
 
         let chevronFace: ChevronFace;
-        if (props.left) {
-            position.left = props.left;
+        if (left) {
+            position.left = left;
             chevronFace = ChevronFace.Left;
         } else {
-            position.right = props.right;
+            position.right = right;
             chevronFace = ChevronFace.Right;
         }
 
         const contextMenuRect = this.state.contextMenuElem ? this.state.contextMenuElem.getBoundingClientRect() : null;
 
         const chevronOffset: CSSProperties = {};
-        if (props.chevronFace) {
-            chevronFace = props.chevronFace;
+        if (propsChevronFace) {
+            chevronFace = propsChevronFace;
         }
         const hasChevron = chevronFace && chevronFace !== ChevronFace.None;
 
         if (chevronFace === ChevronFace.Top || chevronFace === ChevronFace.Bottom) {
-            chevronOffset.left = props.chevronOffset;
+            chevronOffset.left = propsChevronOffset;
         } else {
-            chevronOffset.top = props.chevronOffset;
+            chevronOffset.top = propsChevronOffset;
         }
 
         // If we know the dimensions of the context menu, adjust its position to
@@ -262,39 +284,39 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
         if (contextMenuRect) {
             if (position.top !== undefined) {
                 let maxTop = windowHeight - WINDOW_PADDING;
-                if (!this.props.bottomAligned) {
+                if (!bottomAligned) {
                     maxTop -= contextMenuRect.height;
                 }
                 position.top = Math.min(position.top, maxTop);
                 // Adjust the chevron if necessary
                 if (chevronOffset.top !== undefined) {
-                    chevronOffset.top = props.chevronOffset + props.top - position.top;
+                    chevronOffset.top = propsChevronOffset + 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 + position.bottom - props.bottom;
+                    chevronOffset.top = propsChevronOffset + position.bottom - bottom;
                 }
             }
             if (position.left !== undefined) {
                 let maxLeft = windowWidth - WINDOW_PADDING;
-                if (!this.props.rightAligned) {
+                if (!rightAligned) {
                     maxLeft -= contextMenuRect.width;
                 }
                 position.left = Math.min(position.left, maxLeft);
                 if (chevronOffset.left !== undefined) {
-                    chevronOffset.left = props.chevronOffset + props.left - position.left;
+                    chevronOffset.left = propsChevronOffset + 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 + position.right - props.right;
+                    chevronOffset.left = propsChevronOffset + position.right - right;
                 }
             }
         }
@@ -320,36 +342,36 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
             'mx_ContextualMenu_withChevron_right': chevronFace === ChevronFace.Right,
             'mx_ContextualMenu_withChevron_top': chevronFace === ChevronFace.Top,
             'mx_ContextualMenu_withChevron_bottom': chevronFace === ChevronFace.Bottom,
-            'mx_ContextualMenu_rightAligned': this.props.rightAligned === true,
-            'mx_ContextualMenu_bottomAligned': this.props.bottomAligned === true,
-        }, this.props.menuClassName);
+            'mx_ContextualMenu_rightAligned': rightAligned === true,
+            'mx_ContextualMenu_bottomAligned': bottomAligned === true,
+        }, menuClassName);
 
         const menuStyle: CSSProperties = {};
-        if (props.menuWidth) {
-            menuStyle.width = props.menuWidth;
+        if (menuWidth) {
+            menuStyle.width = menuWidth;
         }
 
-        if (props.menuHeight) {
-            menuStyle.height = props.menuHeight;
+        if (menuHeight) {
+            menuStyle.height = menuHeight;
         }
 
-        if (!isNaN(Number(props.menuPaddingTop))) {
-            menuStyle["paddingTop"] = props.menuPaddingTop;
+        if (!isNaN(Number(menuPaddingTop))) {
+            menuStyle["paddingTop"] = menuPaddingTop;
         }
-        if (!isNaN(Number(props.menuPaddingLeft))) {
-            menuStyle["paddingLeft"] = props.menuPaddingLeft;
+        if (!isNaN(Number(menuPaddingLeft))) {
+            menuStyle["paddingLeft"] = menuPaddingLeft;
         }
-        if (!isNaN(Number(props.menuPaddingBottom))) {
-            menuStyle["paddingBottom"] = props.menuPaddingBottom;
+        if (!isNaN(Number(menuPaddingBottom))) {
+            menuStyle["paddingBottom"] = menuPaddingBottom;
         }
-        if (!isNaN(Number(props.menuPaddingRight))) {
-            menuStyle["paddingRight"] = props.menuPaddingRight;
+        if (!isNaN(Number(menuPaddingRight))) {
+            menuStyle["paddingRight"] = menuPaddingRight;
         }
 
         const wrapperStyle = {};
-        if (!isNaN(Number(props.zIndex))) {
-            menuStyle["zIndex"] = props.zIndex + 1;
-            wrapperStyle["zIndex"] = props.zIndex;
+        if (!isNaN(Number(zIndex))) {
+            menuStyle["zIndex"] = zIndex + 1;
+            wrapperStyle["zIndex"] = zIndex;
         }
 
         let background;
@@ -366,10 +388,10 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
 
         let body = <>
             { chevron }
-            { props.children }
+            { children }
         </>;
 
-        if (props.focusLock) {
+        if (focusLock) {
             body = <FocusLock>
                 { body }
             </FocusLock>;
@@ -379,7 +401,7 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
             <RovingTabIndexProvider handleHomeEnd handleUpDown onKeyDown={this.onKeyDown}>
                 { ({ onKeyDownHandler }) => (
                     <div
-                        className={classNames("mx_ContextualMenu_wrapper", this.props.wrapperClassName)}
+                        className={classNames("mx_ContextualMenu_wrapper", wrapperClassName)}
                         style={{ ...position, ...wrapperStyle }}
                         onClick={this.onClick}
                         onKeyDown={onKeyDownHandler}
@@ -390,7 +412,8 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
                             className={menuClasses}
                             style={menuStyle}
                             ref={this.collectContextMenuRect}
-                            role={this.props.managed ? "menu" : undefined}
+                            role={managed ? "menu" : undefined}
+                            {...props}
                         >
                             { body }
                         </div>
diff --git a/src/components/views/emojipicker/EmojiPicker.tsx b/src/components/views/emojipicker/EmojiPicker.tsx
@@ -240,7 +240,7 @@ class EmojiPicker extends React.Component<IProps, IState> {
     render() {
         let heightBefore = 0;
         return (
-            <div className="mx_EmojiPicker">
+            <div className="mx_EmojiPicker" data-testid='mx_EmojiPicker'>
                 <Header categories={this.categories} onAnchorClick={this.scrollToCategory} />
                 <Search query={this.state.filter} onChange={this.onChangeFilter} onEnter={this.onEnterFilter} />
                 <AutoHideScrollbar
diff --git a/src/components/views/emojipicker/ReactionPicker.tsx b/src/components/views/emojipicker/ReactionPicker.tsx
@@ -134,7 +134,6 @@ class ReactionPicker extends React.Component<IProps, IState> {
             isEmojiDisabled={this.isEmojiDisabled}
             selectedEmojis={this.state.selectedEmojis}
             showQuickReactions={true}
-            data-testid='mx_ReactionPicker'
         />;
     }
 }
diff --git a/src/components/views/messages/MessageActionBar.tsx b/src/components/views/messages/MessageActionBar.tsx
@@ -16,7 +16,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import React, { ReactElement, useContext, useEffect } from 'react';
+import React, { ReactElement, useCallback, useContext, useEffect } from 'react';
 import { EventStatus, MatrixEvent, MatrixEventEvent } from 'matrix-js-sdk/src/models/event';
 import classNames from 'classnames';
 import { MsgType, RelationType } from 'matrix-js-sdk/src/@types/event';
@@ -88,7 +88,7 @@ const OptionsButton: React.FC<IOptionsButtonProps> = ({
         onFocusChange(menuDisplayed);
     }, [onFocusChange, menuDisplayed]);
 
-    const onOptionsClick = (e: React.MouseEvent): void => {
+    const onOptionsClick = useCallback((e: React.MouseEvent): void => {
         // Don't open the regular browser or our context menu on right-click
         e.preventDefault();
         e.stopPropagation();
@@ -97,7 +97,7 @@ const OptionsButton: React.FC<IOptionsButtonProps> = ({
         // the element that is currently focused is skipped. So we want to call onFocus manually to keep the
         // position in the page even when someone is clicking around.
         onFocus();
-    };
+    }, [openMenu, onFocus]);
 
     let contextMenu: ReactElement | null;
     if (menuDisplayed) {
@@ -121,6 +121,7 @@ const OptionsButton: React.FC<IOptionsButtonProps> = ({
             className="mx_MessageActionBar_iconButton mx_MessageActionBar_optionsButton"
             title={_t("Options")}
             onClick={onOptionsClick}
+            onContextMenu={onOptionsClick}
             isExpanded={menuDisplayed}
             inputRef={ref}
             onFocus={onFocus}
@@ -153,17 +154,24 @@ const ReactButton: React.FC<IReactButtonProps> = ({ mxEvent, reactions, onFocusC
         </ContextMenu>;
     }
 
+    const onClick = useCallback((e: React.MouseEvent) => {
+        // Don't open the regular browser or our context menu on right-click
+        e.preventDefault();
+        e.stopPropagation();
+
+        openMenu();
+        // when the context menu is opened directly, e.g. via mouse click, the onFocus handler which tracks
+        // the element that is currently focused is skipped. So we want to call onFocus manually to keep the
+        // position in the page even when someone is clicking around.
+        onFocus();
+    }, [openMenu, onFocus]);
+
     return <React.Fragment>
         <ContextMenuTooltipButton
             className="mx_MessageActionBar_iconButton"
             title={_t("React")}
-            onClick={() => {
-                openMenu();
-                // when the context menu is opened directly, e.g. via mouse click, the onFocus handler which tracks
-                // the element that is currently focused is skipped. So we want to call onFocus manually to keep the
-                // position in the page even when someone is clicking around.
-                onFocus();
-            }}
+            onClick={onClick}
+            onContextMenu={onClick}
             isExpanded={menuDisplayed}
             inputRef={ref}
             onFocus={onFocus}
@@ -193,7 +201,11 @@ const ReplyInThreadButton = ({ mxEvent }: IReplyInThreadButton) => {
         return null;
     }
 
-    const onClick = (): void => {
+    const onClick = (e: React.MouseEvent): void => {
+        // Don't open the regular browser or our context menu on right-click
+        e.preventDefault();
+        e.stopPropagation();
+
         if (firstTimeSeeingThreads) {
             localStorage.setItem("mx_seen_feature_thread", "true");
         }
@@ -245,6 +257,7 @@ const ReplyInThreadButton = ({ mxEvent }: IReplyInThreadButton) => {
             : _t("Can't create a thread from an event with an existing relation")}
 
         onClick={onClick}
+        onContextMenu={onClick}
     >
         <ThreadIcon />
         { firstTimeSeeingThreads && !threadsEnabled && (
@@ -265,10 +278,19 @@ const FavouriteButton = ({ mxEvent }: IFavouriteButtonProp) => {
         'mx_MessageActionBar_favouriteButton_fillstar': isFavourite(eventId),
     });
 
+    const onClick = useCallback((e: React.MouseEvent) => {
+        // Don't open the regular browser or our context menu on right-click
+        e.preventDefault();
+        e.stopPropagation();
+
+        toggleFavourite(eventId);
+    }, [toggleFavourite, eventId]);
+
     return <RovingAccessibleTooltipButton
         className={classes}
         title={_t("Favourite")}
-        onClick={() => toggleFavourite(eventId)}
+        onClick={onClick}
+        onContextMenu={onClick}
         data-testid={eventId}
     >
         <StarIcon />
@@ -335,15 +357,23 @@ export default class MessageActionBar extends React.PureComponent<IMessageAction
         this.props.onFocusChange?.(focused);
     };
 
-    private onReplyClick = (ev: React.MouseEvent): void => {
+    private onReplyClick = (e: React.MouseEvent): void => {
+        // Don't open the regular browser or our context menu on right-click
+        e.preventDefault();
+        e.stopPropagation();
+
         dis.dispatch({
             action: 'reply_to_event',
             event: this.props.mxEvent,
             context: this.context.timelineRenderingType,
         });
     };
 
-    private onEditClick = (): void => {
+    private onEditClick = (e: React.MouseEvent): void => {
+        // Don't open the regular browser or our context menu on right-click
+        e.preventDefault();
+        e.stopPropagation();
+
         editEvent(this.props.mxEvent, this.context.timelineRenderingType, this.props.getRelationsForEvent);
     };
 
@@ -406,6 +436,10 @@ export default class MessageActionBar extends React.PureComponent<IMessageAction
     }
 
     private onResendClick = (ev: React.MouseEvent): void => {
+        // Don't open the regular browser or our context menu on right-click
+        ev.preventDefault();
+        ev.stopPropagation();
+
         this.runActionOnFailedEv((tarEv) => Resend.resend(tarEv));
     };
 
@@ -423,6 +457,7 @@ export default class MessageActionBar extends React.PureComponent<IMessageAction
                 className="mx_MessageActionBar_iconButton"
                 title={_t("Edit")}
                 onClick={this.onEditClick}
+                onContextMenu={this.onEditClick}
                 key="edit"
             >
                 <EditIcon />
@@ -433,6 +468,7 @@ export default class MessageActionBar extends React.PureComponent<IMessageAction
             className="mx_MessageActionBar_iconButton"
             title={_t("Delete")}
             onClick={this.onCancelClick}
+            onContextMenu={this.onCancelClick}
             key="cancel"
         >
             <TrashcanIcon />
@@ -453,6 +489,7 @@ export default class MessageActionBar extends React.PureComponent<IMessageAction
                 className="mx_MessageActionBar_iconButton"
                 title={_t("Retry")}
                 onClick={this.onResendClick}
+                onContextMenu={this.onResendClick}
                 key="resend"
             >
                 <ResendIcon />
@@ -475,6 +512,7 @@ export default class MessageActionBar extends React.PureComponent<IMessageAction
                             className="mx_MessageActionBar_iconButton"
                             title={_t("Reply")}
                             onClick={this.onReplyClick}
+                            onContextMenu={this.onReplyClick}
                             key="reply"
                         >
                             <ReplyIcon />
diff --git a/test/components/views/context_menus/__snapshots__/SpaceContextMenu-test.tsx.snap b/test/components/views/context_menus/__snapshots__/SpaceContextMenu-test.tsx.snap
diff --git a/test/components/views/messages/MessageActionBar-test.tsx b/test/components/views/messages/MessageActionBar-test.tsx

Original file line number	Diff line number	Diff line change
`@@ -134,7 +134,6 @@ class ReactionPicker extends React.Component<IProps, IState> {`
`134`	`134`	`isEmojiDisabled={this.isEmojiDisabled}`
`135`	`135`	`selectedEmojis={this.state.selectedEmojis}`
`136`	`136`	`showQuickReactions={true}`
`137`		`- data-testid='mx_ReactionPicker'`
`138`	`137`	`/>;`
`139`	`138`	`}`
`140`	`139`	`}`