From 19559a62ef2b0a3654a6b1270fdf998b102d5c69 Mon Sep 17 00:00:00 2001
From: Trevor Burch <Trevor.Burch@blackbaud.com>
Date: Fri, 17 Aug 2018 16:05:42 -0400
Subject: [PATCH] Popover hover close fix (#1884)

Resolves #1816 - issues with hover state for popover
---
 src/modules/popover/popover.component.ts      |  1 +
 src/modules/popover/popover.directive.spec.ts | 21 ++++--------
 src/modules/popover/popover.directive.ts      | 32 ++++++++++++-------
 3 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/src/modules/popover/popover.component.ts b/src/modules/popover/popover.component.ts
index 46cbf0639..059c538e6 100644
--- a/src/modules/popover/popover.component.ts
+++ b/src/modules/popover/popover.component.ts
@@ -198,6 +198,7 @@ export class SkyPopoverComponent implements OnInit, OnDestroy {
     }
   }
 
+  // TODO: This method is no longer used. Remove it when we decide to make breaking changes.
   public markForCloseOnMouseLeave() {
     this.isMarkedForCloseOnMouseLeave = true;
   }
diff --git a/src/modules/popover/popover.directive.spec.ts b/src/modules/popover/popover.directive.spec.ts
index b27fa967e..2e98a8037 100644
--- a/src/modules/popover/popover.directive.spec.ts
+++ b/src/modules/popover/popover.directive.spec.ts
@@ -147,32 +147,25 @@ describe('SkyPopoverDirective', () => {
   it('should mark the popover to close on mouseleave', () => {
     const caller = directiveElements[2];
     const callerInstance = caller.injector.get(SkyPopoverDirective);
-    const popoverSpy = spyOn(callerInstance.skyPopover, 'markForCloseOnMouseLeave');
     const closeSpy = spyOn((callerInstance as any), 'closePopover').and.callThrough();
+    const markForCloseSpy = spyOn((callerInstance as any).skyPopover, 'markForCloseOnMouseLeave').and.callThrough();
 
     callerInstance.skyPopover.isOpen = true;
-    callerInstance.skyPopover.isMouseEnter = true;
 
     SkyAppTestUtility.fireDomEvent(caller.nativeElement, 'mouseleave');
-    expect(popoverSpy).not.toHaveBeenCalled();
     expect(closeSpy).toHaveBeenCalled();
+    expect(markForCloseSpy).not.toHaveBeenCalled();
 
-    popoverSpy.calls.reset();
     closeSpy.calls.reset();
+    markForCloseSpy.calls.reset();
 
     // Else path, popover has mouseenter.
-    callerInstance.skyPopover.isOpen = true;
-    spyOn(mockWindowService, 'getWindow').and.returnValue({
-      setTimeout(callback: Function) {
-        // Simulate the popover triggering mouseenter event:
-        callerInstance.skyPopover.isMouseEnter = true;
-        callback();
-      }
-    });
-
+    callerInstance.skyPopover.isOpen = false;
     SkyAppTestUtility.fireDomEvent(caller.nativeElement, 'mouseleave');
-    expect(popoverSpy).toHaveBeenCalledWith();
+    callerInstance.skyPopover.isMouseEnter = true;
+    callerInstance.skyPopover.popoverOpened.emit();
     expect(closeSpy).not.toHaveBeenCalled();
+    expect(markForCloseSpy).toHaveBeenCalled();
   });
 
   it('should close the popover when the escape key is pressed', () => {
diff --git a/src/modules/popover/popover.directive.ts b/src/modules/popover/popover.directive.ts
index b37b2c4c6..fb69b5cf8 100644
--- a/src/modules/popover/popover.directive.ts
+++ b/src/modules/popover/popover.directive.ts
@@ -82,6 +82,14 @@ export class SkyPopoverDirective implements OnChanges, OnDestroy {
     this.skyPopover.close();
   }
 
+  private closePopoverOrMarkForClose() {
+    if (this.skyPopover.isMouseEnter) {
+      this.skyPopover.markForCloseOnMouseLeave();
+    } else {
+      this.closePopover();
+    }
+  }
+
   private isPopoverOpen(): boolean {
     return (this.skyPopover && this.skyPopover.isOpen);
   }
@@ -129,17 +137,19 @@ export class SkyPopoverDirective implements OnChanges, OnDestroy {
         if (this.skyPopoverTrigger === 'mouseenter') {
           event.preventDefault();
 
-          // Give the popover a chance to set its isMouseEnter flag before checking to see
-          // if it should be closed.
-          this.windowRef.getWindow().setTimeout(() => {
-            if (this.isPopoverOpen()) {
-              if (this.skyPopover.isMouseEnter) {
-                this.skyPopover.markForCloseOnMouseLeave();
-              } else {
-                this.closePopover();
-              }
-            }
-          });
+          if (this.isPopoverOpen()) {
+            // Give the popover a chance to set its isMouseEnter flag before checking to see
+            // if it should be closed.
+            this.windowRef.getWindow().setTimeout(() => {
+              this.closePopoverOrMarkForClose();
+            });
+          } else {
+            // If the mouse leaves before the popover is open,
+            // wait for the transition to complete before closing it.
+            this.skyPopover.popoverOpened.take(1).subscribe(() => {
+              this.closePopoverOrMarkForClose();
+            });
+          }
         }
       });
   }