Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

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

Merged
merged 8 commits into from
Aug 18, 2022
Merged

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

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3603001
Fix context menu being opened when clicking buttons
SimonBrandner Aug 17, 2022
2d68958
Correct usage of props
SimonBrandner Aug 17, 2022
281efec
Add tests for right-clicking buttons
SimonBrandner Aug 17, 2022
a1e4f2d
Change how fix the thing
SimonBrandner Aug 17, 2022
424ce46
Make sure tests actually test the thing
SimonBrandner Aug 17, 2022
879d28c
Fix usage of `findByTestId()`
SimonBrandner Aug 17, 2022
73c6e5a
Update snap
SimonBrandner Aug 17, 2022
aa8daad
Remove blank line
SimonBrandner Aug 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 63 additions & 40 deletions src/components/structures/ContextMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
}
}
Expand All @@ -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;
Expand All @@ -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>;
Expand All @@ -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}
Expand All @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/emojipicker/EmojiPicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/emojipicker/ReactionPicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class ReactionPicker extends React.Component<IProps, IState> {
isEmojiDisabled={this.isEmojiDisabled}
selectedEmojis={this.state.selectedEmojis}
showQuickReactions={true}
data-testid='mx_ReactionPicker'

t3chguy marked this conversation as resolved.
Show resolved Hide resolved
/>;
}
}
Expand Down
66 changes: 52 additions & 14 deletions src/components/views/messages/MessageActionBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
Expand All @@ -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) {
Expand All @@ -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}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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 && (
Expand All @@ -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 />
Expand Down Expand Up @@ -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);
};

Expand Down Expand Up @@ -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));
};

Expand All @@ -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 />
Expand All @@ -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 />
Expand All @@ -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 />
Expand All @@ -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 />
Expand Down
Loading