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

This commit is contained in:
Šimon Brandner 2022-08-18 09:18:18 +02:00 committed by GitHub
parent e269c6895d
commit 3eecd68175
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 155 additions and 59 deletions

View file

@ -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;
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 (props.top) {
position.top = props.top;
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,13 +284,13 @@ 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(
@ -276,17 +298,17 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
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(
@ -294,7 +316,7 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
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>

View file

@ -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

View file

@ -134,7 +134,6 @@ class ReactionPicker extends React.Component<IProps, IState> {
isEmojiDisabled={this.isEmojiDisabled}
selectedEmojis={this.state.selectedEmojis}
showQuickReactions={true}
data-testid='mx_ReactionPicker'
/>;
}
}

View file

@ -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>;
}
return <React.Fragment>
<ContextMenuTooltipButton
className="mx_MessageActionBar_iconButton"
title={_t("React")}
onClick={() => {
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={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,7 +357,11 @@ 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,
@ -343,7 +369,11 @@ export default class MessageActionBar extends React.PureComponent<IMessageAction
});
};
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 />

View file

@ -194,6 +194,8 @@ exports[`<SpaceContextMenu /> renders menu correctly 1`] = `
/>
<div
className="mx_ContextualMenu"
hasBackground={true}
onFinished={[MockFunction]}
role="menu"
style={Object {}}
>

View file

@ -240,11 +240,11 @@ describe('<MessageActionBar />', () => {
});
it('opens message context menu on click', () => {
const { findByTestId, queryByLabelText } = getComponent({ mxEvent: alicesMessageEvent });
const { getByTestId, queryByLabelText } = getComponent({ mxEvent: alicesMessageEvent });
act(() => {
fireEvent.click(queryByLabelText('Options'));
});
expect(findByTestId('mx_MessageContextMenu')).toBeTruthy();
expect(getByTestId('mx_MessageContextMenu')).toBeTruthy();
});
});
@ -310,11 +310,11 @@ describe('<MessageActionBar />', () => {
});
it('opens reaction picker on click', () => {
const { queryByLabelText, findByTestId } = getComponent({ mxEvent: alicesMessageEvent });
const { queryByLabelText, getByTestId } = getComponent({ mxEvent: alicesMessageEvent });
act(() => {
fireEvent.click(queryByLabelText('React'));
});
expect(findByTestId('mx_ReactionPicker')).toBeTruthy();
expect(getByTestId('mx_EmojiPicker')).toBeTruthy();
});
});
@ -565,4 +565,38 @@ describe('<MessageActionBar />', () => {
});
});
});
it.each([
["React"],
["Reply"],
["Reply in thread"],
["Favourite"],
["Edit"],
])("does not show context menu when right-clicking", (buttonLabel: string) => {
// For favourite button
jest.spyOn(SettingsStore, 'getValue').mockReturnValue(true);
const event = new MouseEvent("contextmenu", {
bubbles: true,
cancelable: true,
});
event.stopPropagation = jest.fn();
event.preventDefault = jest.fn();
const { queryByTestId, queryByLabelText } = getComponent({ mxEvent: alicesMessageEvent });
act(() => {
fireEvent(queryByLabelText(buttonLabel), event);
});
expect(event.stopPropagation).toHaveBeenCalled();
expect(event.preventDefault).toHaveBeenCalled();
expect(queryByTestId("mx_MessageContextMenu")).toBeFalsy();
});
it("does shows context menu when right-clicking options", () => {
const { queryByTestId, queryByLabelText } = getComponent({ mxEvent: alicesMessageEvent });
act(() => {
fireEvent.contextMenu(queryByLabelText("Options"));
});
expect(queryByTestId("mx_MessageContextMenu")).toBeTruthy();
});
});