From 0b08400a892e036839fb7c9f2ecbe06755c0e21c Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Tue, 14 Sep 2021 00:48:01 +0530 Subject: [PATCH 1/3] fix: message highlight issue --- .../ContextMenu/PopoverReportActionContextMenu.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js b/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js index f2f9e2e89ae7..73a59b17f66e 100644 --- a/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js +++ b/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js @@ -38,6 +38,7 @@ class PopoverReportActionContextMenu extends React.Component { }, }; this.onPopoverHide = () => {}; + this.onPopoverHideActionCallback = () => {}; this.contextMenuAnchor = undefined; this.showContextMenu = this.showContextMenu.bind(this); this.hideContextMenu = this.hideContextMenu.bind(this); @@ -163,18 +164,20 @@ class PopoverReportActionContextMenu extends React.Component { */ runAndResetOnPopoverHide() { this.onPopoverHide(); + this.onPopoverHideActionCallback(); // After we have called the action, reset it. this.onPopoverHide = () => {}; + this.onPopoverHideActionCallback = () => {}; } /** * Hide the ReportActionContextMenu modal popover. - * @param {Function} onHideCallback Callback to be called after popover is completely hidden + * @param {Function} onHideActionCallback Callback to be called after popover is completely hidden */ - hideContextMenu(onHideCallback) { - if (_.isFunction(onHideCallback)) { - this.onPopoverHide = onHideCallback; + hideContextMenu(onHideActionCallback) { + if (_.isFunction(onHideActionCallback)) { + this.onPopoverHideActionCallback = onHideActionCallback; } this.setState({ reportID: 0, From d1ec6b83ddd64a3f410000371bb30b3837a6c75e Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Tue, 14 Sep 2021 01:31:07 +0530 Subject: [PATCH 2/3] Fix: Mini menu hides forever on double right click --- .../report/ContextMenu/PopoverReportActionContextMenu.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js b/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js index 73a59b17f66e..96bf8028951b 100644 --- a/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js +++ b/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js @@ -118,7 +118,10 @@ class PopoverReportActionContextMenu extends React.Component { ) { const nativeEvent = event.nativeEvent || {}; this.contextMenuAnchor = contextMenuAnchor; - this.onPopoverHide = onHide; + const onShowCallback = () => { + onShow(); + this.onPopoverHide = onHide; + }; this.getContextMenuMeasuredLocation().then(({x, y}) => { this.setState({ cursorRelativePosition: { @@ -135,7 +138,7 @@ class PopoverReportActionContextMenu extends React.Component { selection, isPopoverVisible: true, reportActionDraftMessage: draftMessage, - }, onShow); + }, onShowCallback); }); } From dc5020db5a979a4f8bb205483be733fd457097d3 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Fri, 17 Sep 2021 05:48:53 +0530 Subject: [PATCH 3/3] manage race conditions --- .../PopoverReportActionContextMenu.js | 25 ++++++++++++++++--- .../ContextMenu/ReportActionContextMenu.js | 11 +++++++- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js b/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js index 96bf8028951b..ad8846eb6006 100644 --- a/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js +++ b/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js @@ -37,6 +37,7 @@ class PopoverReportActionContextMenu extends React.Component { vertical: 0, }, }; + this.onPopoverShow = () => {}; this.onPopoverHide = () => {}; this.onPopoverHideActionCallback = () => {}; this.contextMenuAnchor = undefined; @@ -47,6 +48,7 @@ class PopoverReportActionContextMenu extends React.Component { this.confirmDeleteAndHideModal = this.confirmDeleteAndHideModal.bind(this); this.hideDeleteModal = this.hideDeleteModal.bind(this); this.showDeleteModal = this.showDeleteModal.bind(this); + this.runAndResetOnPopoverShow = this.runAndResetOnPopoverShow.bind(this); this.runAndResetOnPopoverHide = this.runAndResetOnPopoverHide.bind(this); this.getContextMenuMeasuredLocation = this.getContextMenuMeasuredLocation.bind(this); this.isActiveReportAction = this.isActiveReportAction.bind(this); @@ -118,7 +120,13 @@ class PopoverReportActionContextMenu extends React.Component { ) { const nativeEvent = event.nativeEvent || {}; this.contextMenuAnchor = contextMenuAnchor; - const onShowCallback = () => { + + // Singleton behaviour of ContextMenu creates race conditions when user requests multiple contextMenus. + // But it is possible that every new request registers new callbacks thus instanceID is used to corelate those callbacks + this.instanceID = Math.random().toString(36).substr(2, 5); + + // Register the onHide callback only when Popover is shown to remove the race conditions when there are mutltiple popover open requests + this.onPopoverShow = () => { onShow(); this.onPopoverHide = onHide; }; @@ -138,7 +146,7 @@ class PopoverReportActionContextMenu extends React.Component { selection, isPopoverVisible: true, reportActionDraftMessage: draftMessage, - }, onShowCallback); + }); }); } @@ -163,7 +171,17 @@ class PopoverReportActionContextMenu extends React.Component { } /** - * After Popover hides, call the registered onPopoverHide callback and reset it + * After Popover shows, call the registered onPopoverShow callback and reset it + */ + runAndResetOnPopoverShow() { + this.onPopoverShow(); + + // After we have called the action, reset it. + this.onPopoverShow = () => {}; + } + + /** + * After Popover hides, call the registered onPopoverHide & onPopoverHideActionCallback callback and reset it */ runAndResetOnPopoverHide() { this.onPopoverHide(); @@ -236,6 +254,7 @@ class PopoverReportActionContextMenu extends React.Component { {}) { return; } - setTimeout(() => contextMenuRef.current.hideContextMenu(onHideCallback), 800); + + // Save the active instanceID for which hide action was called. + // If menu is being closed with a delay, check that whether the same instance exists or a new was created. + // If instance is not same, cancel the hide action + const instanceID = contextMenuRef.current.instanceID; + setTimeout(() => { + if (contextMenuRef.current.instanceID === instanceID) { + contextMenuRef.current.hideContextMenu(onHideCallback); + } + }, 800); } function hideDeleteModal() {