Skip to content

Commit

Permalink
[select] fix(Select2): check shouldDismissPopover on menu items (#6070)
Browse files Browse the repository at this point in the history
  • Loading branch information
PatrickBrown1 authored Apr 17, 2023
1 parent 75f8268 commit 6dab069
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 3 deletions.
14 changes: 12 additions & 2 deletions packages/select/src/components/select/select2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,13 @@ import {
setRef,
Utils,
} from "@blueprintjs/core";
import { Popover2, Popover2ClickTargetHandlers, Popover2TargetProps, PopupKind } from "@blueprintjs/popover2";
import {
Popover2,
Classes as Popover2Classes,
Popover2ClickTargetHandlers,
Popover2TargetProps,
PopupKind,
} from "@blueprintjs/popover2";

import { Classes, ListItemsProps, SelectPopoverProps } from "../../common";
import { QueryList, QueryListRendererProps } from "../query-list/queryList";
Expand Down Expand Up @@ -291,7 +297,11 @@ export class Select2<T> extends AbstractPureComponent2<Select2Props<T>, Select2S
};

private handleItemSelect = (item: T, event?: React.SyntheticEvent<HTMLElement>) => {
this.setState({ isOpen: false });
const target = event?.target as HTMLElement;
const shouldDismiss =
target?.closest(`.${CoreClasses.MENU_ITEM}`)?.classList?.contains(Popover2Classes.POPOVER2_DISMISS) ?? true;

this.setState({ isOpen: !shouldDismiss });
this.props.onItemSelect?.(item, event);
};

Expand Down
38 changes: 37 additions & 1 deletion packages/select/test/select2Tests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import * as React from "react";
import * as sinon from "sinon";

import { InputGroup, Keys, MenuItem } from "@blueprintjs/core";
import { Popover2 } from "@blueprintjs/popover2";
import { MenuItem2, Popover2 } from "@blueprintjs/popover2";

import { ItemRendererProps, Select2, Select2Props, Select2State } from "../src";
import { Film, renderFilm, TOP_100_FILMS } from "../src/__examples__";
Expand Down Expand Up @@ -128,6 +128,42 @@ describe("<Select2>", () => {
assert.isTrue(handlers.onItemSelect.calledOnce);
});

it("closes the popover when selecting first MenuItem", () => {
const itemRenderer = (film: Film) => {
return <MenuItem2 text={`${film.rank}. ${film.title}`} shouldDismissPopover={true} />;
};
const wrapper = select({ itemRenderer, popoverProps: { usePortal: false } });

// popover should start close
assert.strictEqual(wrapper.find(Popover2).prop("isOpen"), false);

// popover should open after clicking the button
wrapper.find("[data-testid='target-button']").simulate("click");
assert.strictEqual(wrapper.find(Popover2).prop("isOpen"), true);

// and should close after the a menu item is clicked
wrapper.find(Popover2).find(".bp4-menu-item").first().simulate("click");
assert.strictEqual(wrapper.find(Popover2).prop("isOpen"), false);
});

it("does not close the popover when selecting a MenuItem with shouldDismissPopover", () => {
const itemRenderer = (film: Film) => {
return <MenuItem2 text={`${film.rank}. ${film.title}`} shouldDismissPopover={false} />;
};
const wrapper = select({ itemRenderer, popoverProps: { usePortal: false } });

// popover should start closed
assert.strictEqual(wrapper.find(Popover2).prop("isOpen"), false);

// popover should open after clicking the button
wrapper.find("[data-testid='target-button']").simulate("click");
assert.strictEqual(wrapper.find(Popover2).prop("isOpen"), true);

// and should not close after the a menu item is clicked
wrapper.find(Popover2).find(".bp4-menu-item").first().simulate("click");
assert.strictEqual(wrapper.find(Popover2).prop("isOpen"), true);
});

function select(props: Partial<Select2Props<Film>> = {}, query?: string) {
const wrapper = mount(
<Select2<Film> {...defaultProps} {...handlers} {...props}>
Expand Down

1 comment on commit 6dab069

@adidahiya
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[select] fix(Select2): check shouldDismissPopover on menu items (#6070)

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

Please sign in to comment.