diff --git a/CHANGELOG.md b/CHANGELOG.md index c6656a4ea4..3aab02e9d0 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/desktop/cmp/button/ZoneMapperButton.ts b/desktop/cmp/button/ZoneMapperButton.ts index c3d4833fd4..d78df039f4 100644 --- a/desktop/cmp/button/ZoneMapperButton.ts +++ b/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 { - 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(); } } }); diff --git a/desktop/cmp/grouping/GroupingChooser.ts b/desktop/cmp/grouping/GroupingChooser.ts index ce41c010b5..16ee5789bd 100644 --- a/desktop/cmp/grouping/GroupingChooser.ts +++ b/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(({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), diff --git a/utils/js/DomUtils.ts b/utils/js/DomUtils.ts index e8edb6d1f1..33cbaa139a 100644 --- a/utils/js/DomUtils.ts +++ b/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.