From c74464823b245ec124446c1f2043ae58c2c2dc1b Mon Sep 17 00:00:00 2001 From: David Lechner Date: Thu, 2 Jun 2022 17:35:13 -0500 Subject: [PATCH 1/2] overlay: fix use of deprecated `which` This replaces use of the deprecated keyboard event `which` property with the proper `key` property. Issue: https://github.com/palantir/blueprint/issues/4165 --- packages/core/src/components/overlay/overlay.tsx | 4 +--- packages/core/test/alert/alertTests.tsx | 6 +++--- packages/core/test/dialog/dialogTests.tsx | 3 +-- packages/core/test/drawer/drawerTests.tsx | 3 +-- packages/core/test/overlay/overlayTests.tsx | 5 ++--- packages/core/test/popover/popoverTests.tsx | 3 +-- packages/popover2/test/contextMenu2Tests.tsx | 4 ++-- packages/popover2/test/popover2Tests.tsx | 4 ++-- 8 files changed, 13 insertions(+), 19 deletions(-) diff --git a/packages/core/src/components/overlay/overlay.tsx b/packages/core/src/components/overlay/overlay.tsx index 2e63cbcb96..ab18166773 100644 --- a/packages/core/src/components/overlay/overlay.tsx +++ b/packages/core/src/components/overlay/overlay.tsx @@ -657,9 +657,7 @@ export class Overlay extends AbstractPureComponent2 private handleKeyDown = (e: React.KeyboardEvent) => { const { canEscapeKeyClose, onClose } = this.props; - // HACKHACK: https://github.com/palantir/blueprint/issues/4165 - /* eslint-disable-next-line deprecation/deprecation */ - if (e.which === Keys.ESCAPE && canEscapeKeyClose) { + if (e.key === "Escape" && canEscapeKeyClose) { onClose?.(e); // prevent browser-specific escape key behavior (Safari exits fullscreen) e.preventDefault(); diff --git a/packages/core/test/alert/alertTests.tsx b/packages/core/test/alert/alertTests.tsx index 06692036ec..4a64621ab8 100644 --- a/packages/core/test/alert/alertTests.tsx +++ b/packages/core/test/alert/alertTests.tsx @@ -19,7 +19,7 @@ import { mount, shallow, ShallowWrapper } from "enzyme"; import * as React from "react"; import { SinonStub, spy, stub } from "sinon"; -import { Alert, Button, Classes, IAlertProps, IButtonProps, Icon, Intent, Keys } from "../../src"; +import { Alert, Button, Classes, IAlertProps, IButtonProps, Icon, Intent } from "../../src"; import * as Errors from "../../src/common/errors"; import { findInPortal } from "../utils"; @@ -175,11 +175,11 @@ describe("", () => { ); const overlay = findInPortal(alert, "." + Classes.OVERLAY).first(); - overlay.simulate("keydown", { which: Keys.ESCAPE }); + overlay.simulate("keydown", { key: "Escape" }); assert.isTrue(onCancel.notCalled); alert.setProps({ canEscapeKeyCancel: true }); - overlay.simulate("keydown", { which: Keys.ESCAPE }); + overlay.simulate("keydown", { key: "Escape" }); assert.isTrue(onCancel.calledOnce); alert.unmount(); diff --git a/packages/core/test/dialog/dialogTests.tsx b/packages/core/test/dialog/dialogTests.tsx index 42f12d6f20..eeccee55aa 100644 --- a/packages/core/test/dialog/dialogTests.tsx +++ b/packages/core/test/dialog/dialogTests.tsx @@ -20,7 +20,6 @@ import * as React from "react"; import { spy } from "sinon"; import { Button, Classes, Dialog, DialogProps, H4, Icon, IconSize } from "../../src"; -import * as Keys from "../../src/common/keys"; describe("", () => { it("renders its content correctly", () => { @@ -93,7 +92,7 @@ describe("", () => { {createDialogContents()} , ); - dialog.simulate("keydown", { which: Keys.ESCAPE }); + dialog.simulate("keydown", { key: "Escape" }); assert.isTrue(onClose.notCalled); }); diff --git a/packages/core/test/drawer/drawerTests.tsx b/packages/core/test/drawer/drawerTests.tsx index c621466d9d..87ca5f751a 100644 --- a/packages/core/test/drawer/drawerTests.tsx +++ b/packages/core/test/drawer/drawerTests.tsx @@ -20,7 +20,6 @@ import * as React from "react"; import { spy } from "sinon"; import { Button, Classes, Drawer, DrawerProps, Position } from "../../src"; -import * as Keys from "../../src/common/keys"; describe("", () => { let drawer: ReactWrapper; @@ -197,7 +196,7 @@ describe("", () => { {createDrawerContents()} , ); - drawer.simulate("keydown", { which: Keys.ESCAPE }); + drawer.simulate("keydown", { key: "Escape" }); assert.isTrue(onClose.notCalled); }); diff --git a/packages/core/test/overlay/overlayTests.tsx b/packages/core/test/overlay/overlayTests.tsx index 184b0e07df..4810d4b89b 100644 --- a/packages/core/test/overlay/overlayTests.tsx +++ b/packages/core/test/overlay/overlayTests.tsx @@ -22,7 +22,6 @@ import { spy } from "sinon"; import { dispatchMouseEvent } from "@blueprintjs/test-commons"; import { Classes, Overlay, OverlayProps, Portal, Utils } from "../../src"; -import * as Keys from "../../src/common/keys"; import { findInPortal } from "../utils"; const BACKDROP_SELECTOR = `.${Classes.OVERLAY_BACKDROP}`; @@ -224,7 +223,7 @@ describe("", () => { {createOverlayContents()} , ); - wrapper.simulate("keydown", { which: Keys.ESCAPE }); + wrapper.simulate("keydown", { key: "Escape" }); assert.isTrue(onClose.calledOnce); }); @@ -235,7 +234,7 @@ describe("", () => { {createOverlayContents()} , ); - overlay.simulate("keydown", { which: Keys.ESCAPE }); + overlay.simulate("keydown", { key: "Escape" }); assert.isTrue(onClose.notCalled); overlay.unmount(); }); diff --git a/packages/core/test/popover/popoverTests.tsx b/packages/core/test/popover/popoverTests.tsx index 3acff6bf28..2852bb4c96 100644 --- a/packages/core/test/popover/popoverTests.tsx +++ b/packages/core/test/popover/popoverTests.tsx @@ -24,7 +24,6 @@ import { dispatchMouseEvent } from "@blueprintjs/test-commons"; import { Portal } from "../../src"; import * as Classes from "../../src/common/classes"; import * as Errors from "../../src/common/errors"; -import * as Keys from "../../src/common/keys"; import { Overlay } from "../../src/components/overlay/overlay"; import { IPopoverProps, IPopoverState, Popover, PopoverInteractionKind } from "../../src/components/popover/popover"; import { PopoverArrow } from "../../src/components/popover/popoverArrow"; @@ -833,8 +832,8 @@ describe("", () => { }; wrapper.sendEscapeKey = () => { wrapper!.findClass(Classes.OVERLAY_OPEN).simulate("keydown", { + key: "Escape", nativeEvent: new KeyboardEvent("keydown"), - which: Keys.ESCAPE, }); return wrapper!; }; diff --git a/packages/popover2/test/contextMenu2Tests.tsx b/packages/popover2/test/contextMenu2Tests.tsx index 7bfef02296..7e435e67a2 100644 --- a/packages/popover2/test/contextMenu2Tests.tsx +++ b/packages/popover2/test/contextMenu2Tests.tsx @@ -20,7 +20,7 @@ import { mount, ReactWrapper } from "enzyme"; import * as React from "react"; import { spy } from "sinon"; -import { Classes as CoreClasses, Keys, Menu, MenuItem } from "@blueprintjs/core"; +import { Classes as CoreClasses, Menu, MenuItem } from "@blueprintjs/core"; import { Classes, @@ -80,8 +80,8 @@ describe("ContextMenu2", () => { .find(`.${CoreClasses.OVERLAY_OPEN}`) .hostNodes() .simulate("keydown", { + key: "Escape", nativeEvent: new KeyboardEvent("keydown"), - which: Keys.ESCAPE, }); assert.isFalse(ctxMenu.find(Popover2).prop("isOpen")); }); diff --git a/packages/popover2/test/popover2Tests.tsx b/packages/popover2/test/popover2Tests.tsx index ae82d3a549..e4f1a3c2cd 100644 --- a/packages/popover2/test/popover2Tests.tsx +++ b/packages/popover2/test/popover2Tests.tsx @@ -19,7 +19,7 @@ import { mount, ReactWrapper, shallow } from "enzyme"; import * as React from "react"; import * as sinon from "sinon"; -import { Classes as CoreClasses, Keys, Menu, MenuItem, Overlay, Portal } from "@blueprintjs/core"; +import { Classes as CoreClasses, Menu, MenuItem, Overlay, Portal } from "@blueprintjs/core"; import { dispatchMouseEvent } from "@blueprintjs/test-commons"; import { Classes, Errors } from "../src"; @@ -832,8 +832,8 @@ describe("", () => { }; wrapper.sendEscapeKey = () => { wrapper!.findClass(CoreClasses.OVERLAY_OPEN).simulate("keydown", { + key: "Escape", nativeEvent: new KeyboardEvent("keydown"), - which: Keys.ESCAPE, }); return wrapper!; }; From 35e00988e66eab212eef9e3ccaffbaeb064d8fd8 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Thu, 2 Jun 2022 17:37:26 -0500 Subject: [PATCH 2/2] overlay: stop propagation on escape keydown event This adds a call to e.stopPropagation() to prevent nested overlays from closing on a single key press. Fixes: https://github.com/palantir/blueprint/issues/5099 --- packages/core/src/components/overlay/overlay.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core/src/components/overlay/overlay.tsx b/packages/core/src/components/overlay/overlay.tsx index ab18166773..8825e9dcd9 100644 --- a/packages/core/src/components/overlay/overlay.tsx +++ b/packages/core/src/components/overlay/overlay.tsx @@ -659,6 +659,8 @@ export class Overlay extends AbstractPureComponent2 const { canEscapeKeyClose, onClose } = this.props; if (e.key === "Escape" && canEscapeKeyClose) { onClose?.(e); + // prevent other overlays from closing + e.stopPropagation(); // prevent browser-specific escape key behavior (Safari exits fullscreen) e.preventDefault(); }