[Overlay] Only remove body.pt-overlay-open when the stack is empty (#…

…1369)
palantir · Jul 26, 2017 · 94d68e8 · 94d68e8 · blueprint-bot · Jul 26, 2017
1 parent 85a42b2
commit 94d68e8
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 17 deletions.
diff --git a/packages/core/src/components/overlay/overlay.tsx b/packages/core/src/components/overlay/overlay.tsx
@@ -249,15 +249,17 @@ export class Overlay extends React.Component<IOverlayProps, IOverlayState> {
         document.removeEventListener("focus", this.handleDocumentFocus, /* useCapture */ true);
         document.removeEventListener("mousedown", this.handleDocumentClick);
 
-        document.body.classList.remove(Classes.OVERLAY_OPEN);
-
         const { openStack } = Overlay;
         const stackIndex = openStack.indexOf(this);
         if (stackIndex !== -1) {
             openStack.splice(stackIndex, 1);
-            const lastOpenedOverlay = Overlay.getLastOpened();
-            if (openStack.length > 0 && lastOpenedOverlay.props.enforceFocus) {
-                document.addEventListener("focus", lastOpenedOverlay.handleDocumentFocus, /* useCapture */ true);
+            if (openStack.length > 0) {
+                const lastOpenedOverlay = Overlay.getLastOpened();
+                if (lastOpenedOverlay.props.enforceFocus) {
+                    document.addEventListener("focus", lastOpenedOverlay.handleDocumentFocus, /* useCapture */ true);
+                }
+            } else {
+                document.body.classList.remove(Classes.OVERLAY_OPEN);
             }
         }
     }

diff --git a/packages/core/test/overlay/overlayTests.tsx b/packages/core/test/overlay/overlayTests.tsx
@@ -279,30 +279,35 @@ describe("<Overlay>", () => {
     });
 
     describe("Background scrolling", () => {
+        beforeEach(() => {
+            // force-reset Overlay stack state between tests
+            (Overlay as any).openStack = [];
+            document.body.classList.remove(Classes.OVERLAY_OPEN);
+        });
 
-        it("disables document scrolling by default", () => {
+        it("disables document scrolling by default", (done) => {
             wrapper = mountOverlay(undefined, undefined);
-            assert.isTrue(isBodyScrollingDisabled());
+            assertBodyScrollingDisabled(true, done);
         });
 
-        it("disables document scrolling if inline=false and hasBackdrop=true", () => {
+        it("disables document scrolling if inline=false and hasBackdrop=true", (done) => {
             wrapper = mountOverlay(false, true);
-            assert.isTrue(isBodyScrollingDisabled());
+            assertBodyScrollingDisabled(true, done);
         });
 
-        it("does not disable document scrolling if inline=false and hasBackdrop=false", () => {
+        it("does not disable document scrolling if inline=false and hasBackdrop=false", (done) => {
             wrapper = mountOverlay(false, false);
-            assert.isFalse(isBodyScrollingDisabled());
+            assertBodyScrollingDisabled(false, done);
         });
 
-        it("does not disable document scrolling if inline=true and hasBackdrop=true", () => {
+        it("does not disable document scrolling if inline=true and hasBackdrop=true", (done) => {
             wrapper = mountOverlay(true, true);
-            assert.isFalse(isBodyScrollingDisabled());
+            assertBodyScrollingDisabled(false, done);
         });
 
-        it("does not disable document scrolling if inline=true and hasBackdrop=false", () => {
+        it("does not disable document scrolling if inline=true and hasBackdrop=false", (done) => {
             wrapper = mountOverlay(true, false);
-            assert.isFalse(isBodyScrollingDisabled());
+            assertBodyScrollingDisabled(false, done);
         });
 
         function mountOverlay(inline: boolean, hasBackdrop: boolean) {
@@ -313,8 +318,13 @@ describe("<Overlay>", () => {
             );
         }
 
-        function isBodyScrollingDisabled() {
-            return document.body.classList.contains(Classes.OVERLAY_OPEN);
+        function assertBodyScrollingDisabled(disabled: boolean, done: MochaDone) {
+            // wait for the DOM to settle before checking body classes
+            setTimeout(() => {
+                const hasClass = document.body.classList.contains(Classes.OVERLAY_OPEN);
+                assert.equal(hasClass, disabled);
+                done();
+            });
         }
     });