From 89bd572371e695e861b6d76c23943c1193ff65da Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 1 Jul 2020 23:05:37 +0100 Subject: [PATCH 1/5] Fix context menu nesting causing bubbling and instabilities Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/ContextMenu.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/components/structures/ContextMenu.js b/src/components/structures/ContextMenu.js index 98b0867ccc3..a56a987fcfb 100644 --- a/src/components/structures/ContextMenu.js +++ b/src/components/structures/ContextMenu.js @@ -116,6 +116,7 @@ export class ContextMenu extends React.Component { this.props.onFinished(); e.preventDefault(); + e.stopPropagation(); const x = e.clientX; const y = e.clientY; @@ -133,6 +134,12 @@ export class ContextMenu extends React.Component { } }; + onContextMenuPreventBubbling = (e) => { + // stop propagation so that any context menu handlers don't leak out of this context menu + // but do not inhibit the default browser menu + e.stopPropagation(); + }; + _onMoveFocus = (element, up) => { let descending = false; // are we currently descending or ascending through the DOM tree? @@ -324,7 +331,7 @@ export class ContextMenu extends React.Component { } return ( -
+
{ chevron } { props.children } From 5c2b291510aa2cbacd7785b0d9a37eca0844121f Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 1 Jul 2020 23:06:26 +0100 Subject: [PATCH 2/5] Support right click context menu interactions on Room List 2 Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/UserMenu.tsx | 40 ++++++++---- src/components/views/rooms/RoomSublist2.tsx | 42 ++++++++----- src/components/views/rooms/RoomTile2.tsx | 70 +++++++++++---------- 3 files changed, 92 insertions(+), 60 deletions(-) diff --git a/src/components/structures/UserMenu.tsx b/src/components/structures/UserMenu.tsx index d6771f30119..92a4666a9d2 100644 --- a/src/components/structures/UserMenu.tsx +++ b/src/components/structures/UserMenu.tsx @@ -42,8 +42,10 @@ interface IProps { isMinimized: boolean; } +type PartialDOMRect = Pick; + interface IState { - menuDisplayed: boolean; + contextMenuPosition: PartialDOMRect; isDarkTheme: boolean; } @@ -56,7 +58,7 @@ export default class UserMenu extends React.Component { super(props); this.state = { - menuDisplayed: false, + contextMenuPosition: null, isDarkTheme: this.isUserOnDarkTheme(), }; @@ -106,13 +108,27 @@ export default class UserMenu extends React.Component { private onOpenMenuClick = (ev: InputEvent) => { ev.preventDefault(); ev.stopPropagation(); - this.setState({menuDisplayed: true}); + const target = ev.target as HTMLButtonElement; + this.setState({contextMenuPosition: target.getBoundingClientRect()}); + }; + + private onContextMenu = (ev: React.MouseEvent) => { + ev.preventDefault(); + ev.stopPropagation(); + this.setState({ + contextMenuPosition: { + left: ev.clientX, + top: ev.clientY, + width: 20, + height: 0, + }, + }); }; private onCloseMenu = (ev: InputEvent) => { ev.preventDefault(); ev.stopPropagation(); - this.setState({menuDisplayed: false}); + this.setState({contextMenuPosition: null}); }; private onSwitchThemeClick = () => { @@ -129,7 +145,7 @@ export default class UserMenu extends React.Component { const payload: OpenToTabPayload = {action: Action.ViewUserSettings, initialTabId: tabId}; defaultDispatcher.dispatch(payload); - this.setState({menuDisplayed: false}); // also close the menu + this.setState({contextMenuPosition: null}); // also close the menu }; private onShowArchived = (ev: ButtonEvent) => { @@ -145,7 +161,7 @@ export default class UserMenu extends React.Component { ev.stopPropagation(); Modal.createTrackedDialog('Report bugs & give feedback', '', RedesignFeedbackDialog); - this.setState({menuDisplayed: false}); // also close the menu + this.setState({contextMenuPosition: null}); // also close the menu }; private onSignOutClick = (ev: ButtonEvent) => { @@ -153,7 +169,7 @@ export default class UserMenu extends React.Component { ev.stopPropagation(); Modal.createTrackedDialog('Logout from LeftPanel', '', LogoutDialog); - this.setState({menuDisplayed: false}); // also close the menu + this.setState({contextMenuPosition: null}); // also close the menu }; private onHomeClick = (ev: ButtonEvent) => { @@ -164,7 +180,7 @@ export default class UserMenu extends React.Component { }; private renderContextMenu = (): React.ReactNode => { - if (!this.state.menuDisplayed) return null; + if (!this.state.contextMenuPosition) return null; let hostingLink; const signupLink = getHostingLink("user-context-menu"); @@ -198,13 +214,12 @@ export default class UserMenu extends React.Component { ); } - const elementRect = this.buttonRef.current.getBoundingClientRect(); return (
@@ -290,7 +305,8 @@ export default class UserMenu extends React.Component { onClick={this.onOpenMenuClick} inputRef={this.buttonRef} label={_t("Account settings")} - isExpanded={this.state.menuDisplayed} + isExpanded={!!this.state.contextMenuPosition} + onContextMenu={this.onContextMenu} >
diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 461098cb634..36d38c7087c 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -65,22 +65,21 @@ interface IProps { // TODO: Account for https://github.com/vector-im/riot-web/issues/14179 } +type PartialDOMRect = Pick; + interface IState { notificationState: ListNotificationState; - menuDisplayed: boolean; + contextMenuPosition: PartialDOMRect; isResizing: boolean; } export default class RoomSublist2 extends React.Component { - private headerButton = createRef(); - private menuButtonRef: React.RefObject = createRef(); - constructor(props: IProps) { super(props); this.state = { notificationState: new ListNotificationState(this.props.isInvite, this.props.tagId), - menuDisplayed: false, + contextMenuPosition: null, isResizing: false, }; this.state.notificationState.setRooms(this.props.rooms); @@ -132,11 +131,24 @@ export default class RoomSublist2 extends React.Component { private onOpenMenuClick = (ev: InputEvent) => { ev.preventDefault(); ev.stopPropagation(); - this.setState({menuDisplayed: true}); + const target = ev.target as HTMLButtonElement; + this.setState({contextMenuPosition: target.getBoundingClientRect()}); + }; + + private onContextMenu = (ev: React.MouseEvent) => { + ev.preventDefault(); + ev.stopPropagation(); + this.setState({ + contextMenuPosition: { + left: ev.clientX, + top: ev.clientY, + height: 0, + }, + }); }; private onCloseMenu = () => { - this.setState({menuDisplayed: false}); + this.setState({contextMenuPosition: null}); }; private onUnreadFirstChanged = async () => { @@ -202,15 +214,14 @@ export default class RoomSublist2 extends React.Component { } let contextMenu = null; - if (this.state.menuDisplayed) { - const elementRect = this.menuButtonRef.current.getBoundingClientRect(); + if (this.state.contextMenuPosition) { const isAlphabetical = RoomListStore.instance.getTagSorting(this.props.tagId) === SortAlgorithm.Alphabetic; const isUnreadFirst = RoomListStore.instance.getListOrder(this.props.tagId) === ListAlgorithm.Importance; contextMenu = (
@@ -261,9 +272,8 @@ export default class RoomSublist2 extends React.Component { {contextMenu} @@ -272,7 +282,7 @@ export default class RoomSublist2 extends React.Component { private renderHeader(): React.ReactElement { return ( - + {({onFocus, isActive, ref}) => { // TODO: Use onFocus: https://github.com/vector-im/riot-web/issues/14180 const tabIndex = isActive ? 0 : -1; @@ -317,12 +327,14 @@ export default class RoomSublist2 extends React.Component {
{this.props.label} @@ -347,7 +359,7 @@ export default class RoomSublist2 extends React.Component { const classes = classNames({ 'mx_RoomSublist2': true, - 'mx_RoomSublist2_hasMenuOpen': this.state.menuDisplayed, + 'mx_RoomSublist2_hasMenuOpen': !!this.state.contextMenuPosition, 'mx_RoomSublist2_minimized': this.props.isMinimized, }); diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index c9a1f39982d..47b5b4206ba 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -60,15 +60,17 @@ interface IProps { // TODO: Incoming call boxes: https://github.com/vector-im/riot-web/issues/14177 } +type PartialDOMRect = Pick; + interface IState { hover: boolean; notificationState: INotificationState; selected: boolean; - notificationsMenuDisplayed: boolean; - generalMenuDisplayed: boolean; + notificationsMenuPosition: PartialDOMRect; + generalMenuPosition: PartialDOMRect; } -const contextMenuBelow = (elementRect) => { +const contextMenuBelow = (elementRect: PartialDOMRect) => { // align the context menu's icons with the icon which opened the context menu const left = elementRect.left + window.pageXOffset - 9; let top = elementRect.bottom + window.pageYOffset + 17; @@ -103,9 +105,6 @@ const NotifOption: React.FC = ({active, onClick, iconClassNam }; export default class RoomTile2 extends React.Component { - private notificationsMenuButtonRef: React.RefObject = createRef(); - private generalMenuButtonRef: React.RefObject = createRef(); - // TODO: a11y: https://github.com/vector-im/riot-web/issues/14180 constructor(props: IProps) { @@ -115,8 +114,8 @@ export default class RoomTile2 extends React.Component { hover: false, notificationState: new TagSpecificNotificationState(this.props.room, this.props.tag), selected: ActiveRoomObserver.activeRoomId === this.props.room.roomId, - notificationsMenuDisplayed: false, - generalMenuDisplayed: false, + notificationsMenuPosition: null, + generalMenuPosition: null, }; ActiveRoomObserver.addListener(this.props.room.roomId, this.onActiveRoomUpdate); @@ -137,6 +136,8 @@ export default class RoomTile2 extends React.Component { }; private onTileClick = (ev: React.KeyboardEvent) => { + ev.preventDefault(); + ev.stopPropagation(); dis.dispatch({ action: 'view_room', // TODO: Support show_room_tile in new room list: https://github.com/vector-im/riot-web/issues/14233 @@ -153,25 +154,34 @@ export default class RoomTile2 extends React.Component { private onNotificationsMenuOpenClick = (ev: InputEvent) => { ev.preventDefault(); ev.stopPropagation(); - this.setState({notificationsMenuDisplayed: true}); + const target = ev.target as HTMLButtonElement; + this.setState({notificationsMenuPosition: target.getBoundingClientRect()}); }; - private onCloseNotificationsMenu = (ev: InputEvent) => { - ev.preventDefault(); - ev.stopPropagation(); - this.setState({notificationsMenuDisplayed: false}); + private onCloseNotificationsMenu = () => { + this.setState({notificationsMenuPosition: null}); }; private onGeneralMenuOpenClick = (ev: InputEvent) => { ev.preventDefault(); ev.stopPropagation(); - this.setState({generalMenuDisplayed: true}); + const target = ev.target as HTMLButtonElement; + this.setState({generalMenuPosition: target.getBoundingClientRect()}); }; - private onCloseGeneralMenu = (ev: InputEvent) => { + private onContextMenu = (ev: React.MouseEvent) => { ev.preventDefault(); ev.stopPropagation(); - this.setState({generalMenuDisplayed: false}); + this.setState({ + generalMenuPosition: { + left: ev.clientX, + bottom: ev.clientY, + }, + }); + }; + + private onCloseGeneralMenu = () => { + this.setState({generalMenuPosition: null}); }; private onTagRoom = (ev: ButtonEvent, tagId: TagID) => { @@ -190,7 +200,7 @@ export default class RoomTile2 extends React.Component { action: 'leave_room', room_id: this.props.room.roomId, }); - this.setState({generalMenuDisplayed: false}); // hide the menu + this.setState({generalMenuPosition: null}); // hide the menu }; private onOpenRoomSettings = (ev: ButtonEvent) => { @@ -201,7 +211,7 @@ export default class RoomTile2 extends React.Component { action: 'open_room_settings', room_id: this.props.room.roomId, }); - this.setState({generalMenuDisplayed: false}); // hide the menu + this.setState({generalMenuPosition: null}); // hide the menu }; private async saveNotifState(ev: ButtonEvent, newState: ALL_MESSAGES_LOUD | ALL_MESSAGES | MENTIONS_ONLY | MUTE) { @@ -218,10 +228,7 @@ export default class RoomTile2 extends React.Component { console.error(error); } - // Close the context menu - this.setState({ - notificationsMenuDisplayed: false, - }); + this.setState({notificationsMenuPosition: null}); // Close the context menu } private onClickAllNotifs = ev => this.saveNotifState(ev, ALL_MESSAGES); @@ -238,10 +245,9 @@ export default class RoomTile2 extends React.Component { const state = getRoomNotifsState(this.props.room.roomId); let contextMenu = null; - if (this.state.notificationsMenuDisplayed) { - const elementRect = this.notificationsMenuButtonRef.current.getBoundingClientRect(); + if (this.state.notificationsMenuPosition) { contextMenu = ( - +
{ {contextMenu} @@ -307,10 +312,9 @@ export default class RoomTile2 extends React.Component { } let contextMenu = null; - if (this.state.generalMenuDisplayed) { - const elementRect = this.generalMenuButtonRef.current.getBoundingClientRect(); + if (this.state.generalMenuPosition) { contextMenu = ( - +
this.onTagRoom(e, DefaultTagID.Favourite)}> @@ -338,9 +342,8 @@ export default class RoomTile2 extends React.Component { {contextMenu} @@ -354,7 +357,7 @@ export default class RoomTile2 extends React.Component { const classes = classNames({ 'mx_RoomTile2': true, 'mx_RoomTile2_selected': this.state.selected, - 'mx_RoomTile2_hasMenuOpen': this.state.generalMenuDisplayed || this.state.notificationsMenuDisplayed, + 'mx_RoomTile2_hasMenuOpen': !!(this.state.generalMenuPosition || this.state.notificationsMenuPosition), 'mx_RoomTile2_minimized': this.props.isMinimized, }); @@ -416,6 +419,7 @@ export default class RoomTile2 extends React.Component { onMouseLeave={this.onTileMouseLeave} onClick={this.onTileClick} role="treeitem" + onContextMenu={this.onContextMenu} >
From e8702aafa564030699e81a511aaeebcdd2785df4 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 1 Jul 2020 23:09:02 +0100 Subject: [PATCH 3/5] delint Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/RoomTile2.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index 47b5b4206ba..7a070279130 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -73,7 +73,7 @@ interface IState { const contextMenuBelow = (elementRect: PartialDOMRect) => { // align the context menu's icons with the icon which opened the context menu const left = elementRect.left + window.pageXOffset - 9; - let top = elementRect.bottom + window.pageYOffset + 17; + const top = elementRect.bottom + window.pageYOffset + 17; const chevronFace = "none"; return {left, top, chevronFace}; }; From 6424ffb22a6e99ad61a094c4f8433b42c8d9f93b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 1 Jul 2020 23:13:54 +0100 Subject: [PATCH 4/5] fix repeated context menu interaction by not erroring Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/UserMenu.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/structures/UserMenu.tsx b/src/components/structures/UserMenu.tsx index 92a4666a9d2..1cfe244845d 100644 --- a/src/components/structures/UserMenu.tsx +++ b/src/components/structures/UserMenu.tsx @@ -125,9 +125,7 @@ export default class UserMenu extends React.Component { }); }; - private onCloseMenu = (ev: InputEvent) => { - ev.preventDefault(); - ev.stopPropagation(); + private onCloseMenu = () => { this.setState({contextMenuPosition: null}); }; From 4b27a67e336854f1f634b582ee3670a16a931623 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 1 Jul 2020 23:16:54 +0100 Subject: [PATCH 5/5] improve default behaviour for consistency Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/ContextMenu.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/components/structures/ContextMenu.js b/src/components/structures/ContextMenu.js index a56a987fcfb..5ba26627961 100644 --- a/src/components/structures/ContextMenu.js +++ b/src/components/structures/ContextMenu.js @@ -347,10 +347,18 @@ export class ContextMenu extends React.Component { } // Semantic component for representing the AccessibleButton which launches a -export const ContextMenuButton = ({ label, isExpanded, children, ...props }) => { +export const ContextMenuButton = ({ label, isExpanded, children, onClick, onContextMenu, ...props }) => { const AccessibleButton = sdk.getComponent('elements.AccessibleButton'); return ( - + { children } );