Prevent 'mousedown' events within Select from closing Popovers. (#3519)

* Prvent 'mousedown' events within Select from closing Popovers. #3516

* Added CHANGELOG entry

CHANGELOG.md

@@ -25,6 +25,12 @@
   multi-line full-width rows. Each row is broken into four zones for top/bottom and left/right,
   each of which can mapped to render one or more fields.
 
+### 🐞 Bug Fixes
+
+* Fixed bug where interacting with a `Select` within a `Popover` can inadvertently cause the
+  popover to close. If your app already has special handling in place to prevent this, you should
+  be able to unwind it after upgrading.
+
 ### 💥 Breaking Changes
 
 * The `XH.getActiveModels` method has been renamed to `XH.getModels` for clarity and consistency.

desktop/cmp/button/ZoneMapperButton.ts

@@ -13,7 +13,6 @@ import {zoneMapper} from '@xh/hoist/desktop/cmp/zoneGrid/impl/ZoneMapper';
 import {Icon} from '@xh/hoist/icon';
 import {popover, Position} from '@xh/hoist/kit/blueprint';
 import {stopPropagation, withDefault} from '@xh/hoist/utils/js';
-import {MENU_PORTAL_ID} from '@xh/hoist/desktop/cmp/input';
 import {button, ButtonProps} from './Button';
 
 export interface ZoneMapperButtonProps extends ButtonProps {
@@ -71,18 +70,11 @@ export const [ZoneMapperButton, zoneMapperButton] = hoistCmp.withFactory<ZoneMap
                     zoneMapper({model: mapperModel})
                 ]
             }),
-            onInteraction: (willOpen, e?) => {
-                if (isOpen && !willOpen) {
-                    // Prevent clicks with Select controls from closing popover
-                    const selectPortal = document.getElementById(MENU_PORTAL_ID),
-                        selectPortalClick = selectPortal?.contains(e?.target),
-                        selectValueClick = e?.target?.classList.contains('xh-select__single-value');
-
-                    if (!selectPortalClick && !selectValueClick) {
-                        mapperModel.close();
-                    }
-                } else if (!isOpen && willOpen) {
+            onInteraction: (nextOpenState, e) => {
+                if (nextOpenState) {
                     mapperModel.openPopover();
+                } else {
+                    mapperModel.close();
                 }
             }
         });

desktop/cmp/grouping/GroupingChooser.ts

@@ -8,13 +8,13 @@ import {GroupingChooserModel} from '@xh/hoist/cmp/grouping';
 import {box, div, filler, fragment, hbox, vbox} from '@xh/hoist/cmp/layout';
 import {hoistCmp, uses} from '@xh/hoist/core';
 import {button, ButtonProps} from '@xh/hoist/desktop/cmp/button';
-import {MENU_PORTAL_ID, select} from '@xh/hoist/desktop/cmp/input';
+import {select} from '@xh/hoist/desktop/cmp/input';
 import {panel} from '@xh/hoist/desktop/cmp/panel';
 import '@xh/hoist/desktop/register';
 import {Icon} from '@xh/hoist/icon';
 import {menu, menuDivider, menuItem, popover} from '@xh/hoist/kit/blueprint';
 import {dragDropContext, draggable, droppable} from '@xh/hoist/kit/react-beautiful-dnd';
-import {getTestId, TEST_ID} from '@xh/hoist/utils/js';
+import {elemWithin, getTestId, TEST_ID} from '@xh/hoist/utils/js';
 import {splitLayoutProps} from '@xh/hoist/utils/react';
 import classNames from 'classnames';
 import {compact, isEmpty, sortBy} from 'lodash';
@@ -117,7 +117,7 @@ export const [GroupingChooser, groupingChooser] = hoistCmp.withFactory<GroupingC
                         isOpen &&
                         nextOpenState === false &&
                         e?.target &&
-                        !targetIsControlButtonOrPortal(e.target)
+                        !elemWithin(e.target, 'xh-grouping-chooser-button--with-favorites')
                     ) {
                         model.commitPendingValueAndClose();
                     }
@@ -305,23 +305,6 @@ function getDimOptions(dims, model) {
     return sortBy(ret, 'label');
 }
 
-function targetIsControlButtonOrPortal(target) {
-    const selectPortal = document.getElementById(MENU_PORTAL_ID)?.contains(target),
-        selectClick = targetWithin(target, 'xh-select__single-value'),
-        editorClick = targetWithin(target, 'xh-grouping-chooser-button--with-favorites');
-    return selectPortal || selectClick || editorClick;
-}
-
-/**
- * Determines whether any of the target's parents have a specific class name
- */
-function targetWithin(target, className): boolean {
-    for (let elem = target; elem; elem = elem.parentElement) {
-        if (elem.classList.contains(className)) return true;
-    }
-    return false;
-}
-
 //------------------
 // Favorites
 //------------------

desktop/cmp/input/Select.ts

@@ -28,7 +28,7 @@ import {
 } from '@xh/hoist/kit/react-select';
 import {action, bindable, makeObservable, observable, override} from '@xh/hoist/mobx';
 import {wait} from '@xh/hoist/promise';
-import {getTestId, TEST_ID, throwIf, withDefault} from '@xh/hoist/utils/js';
+import {elemWithin, getTestId, TEST_ID, throwIf, withDefault} from '@xh/hoist/utils/js';
 import {createObservableRef, getLayoutProps} from '@xh/hoist/utils/react';
 import classNames from 'classnames';
 import debouncePromise from 'debounce-promise';
@@ -789,6 +789,15 @@ const cmp = hoistCmp.factory<SelectInputModel>(({model, className, ...props}, re
                 e.stopPropagation();
             }
         },
+        onMouseDown: e => {
+            // Some internal elements, like the dropdown indicator and the rendered single value,
+            // fire 'mousedown' events. These can bubble and inadvertently close Popovers that
+            // contain Selects.
+            const target = e?.target as HTMLElement;
+            if (target && elemWithin(target, 'bp4-popover')) {
+                e.stopPropagation();
+            }
+        },
         testId: props.testId,
         ...layoutProps,
         width: withDefault(width, 200),

utils/js/DomUtils.ts

@@ -91,6 +91,16 @@ export function observeVisibleChange(fn: (visible: boolean) => any, node: Elemen
     return ret;
 }
 
+/**
+ * Determines whether an element has an ancestor with a specific class name
+ */
+export function elemWithin(target: HTMLElement, className: string): boolean {
+    for (let elem = target; elem; elem = elem.parentElement) {
+        if (elem.classList.contains(className)) return true;
+    }
+    return false;
+}
+
 /**
  * A convenience handler that will call 'stopPropagation'
  * and 'preventDefault' on an event.