Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Popover hover close fix #1884

Merged
merged 18 commits into from
Aug 17, 2018
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
0aaf119
Increased timeout for mouseleave event
Blackbaud-TrevorBurch Aug 6, 2018
907ff76
Merge branch 'master' into 1816-popover-hover-timeout
Blackbaud-AlexKingman Aug 7, 2018
1d1e151
Merge branch 'master' into 1816-popover-hover-timeout
Blackbaud-TrevorBurch Aug 10, 2018
2a22113
Merge branch 'master' of https://github.com/blackbaud/skyux2 into 181…
Blackbaud-TrevorBurch Aug 13, 2018
547cf28
Changed code to watch for observable instead of using a hard coded ti…
Blackbaud-TrevorBurch Aug 13, 2018
ad76e9b
Fixed build issue
Blackbaud-TrevorBurch Aug 13, 2018
e8fa068
Fixed test that was broken from change.
Blackbaud-TrevorBurch Aug 15, 2018
e7605e9
Merge branch 'master' into 1816-popover-hover-timeout
Blackbaud-TrevorBurch Aug 15, 2018
d2051a4
Merge branch 'master' into 1816-popover-hover-timeout
Blackbaud-TrevorBurch Aug 15, 2018
b1cbb6d
Merge branch 'master' into 1816-popover-hover-timeout
Blackbaud-TrevorBurch Aug 15, 2018
8d65972
Added TODO to remove popover's markForCloseOnMouseLeave method
Blackbaud-TrevorBurch Aug 16, 2018
262c0e5
Merge branch 'master' into 1816-popover-hover-timeout
Blackbaud-TrevorBurch Aug 16, 2018
6538f14
Fixed issue with hover states and timing
Blackbaud-TrevorBurch Aug 17, 2018
498f842
Merge branch '1816-popover-hover-timeout' of https://github.com/black…
Blackbaud-TrevorBurch Aug 17, 2018
9babb76
Merge branch 'master' into 1816-popover-hover-timeout
Blackbaud-TrevorBurch Aug 17, 2018
45458f6
Fixed typo
Blackbaud-TrevorBurch Aug 17, 2018
8c62f85
Fixed code coverage
Blackbaud-TrevorBurch Aug 17, 2018
fd96954
Merge branch 'master' into 1816-popover-hover-timeout
Blackbaud-TrevorBurch Aug 17, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 3 additions & 14 deletions src/modules/popover/popover.directive.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,32 +147,21 @@ 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();

callerInstance.skyPopover.isOpen = true;
callerInstance.skyPopover.isMouseEnter = true;

SkyAppTestUtility.fireDomEvent(caller.nativeElement, 'mouseleave');
expect(popoverSpy).not.toHaveBeenCalled();
expect(closeSpy).toHaveBeenCalled();

popoverSpy.calls.reset();
closeSpy.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();
expect(closeSpy).not.toHaveBeenCalled();
callerInstance.skyPopover.popoverOpened.emit();
expect(closeSpy).toHaveBeenCalled();
});

it('should close the popover when the escape key is pressed', () => {
Expand Down
29 changes: 11 additions & 18 deletions src/modules/popover/popover.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@ import 'rxjs/add/observable/fromEvent';
import { Subject } from 'rxjs/Subject';
import 'rxjs/add/operator/takeUntil';

import {
SkyWindowRefService
} from '../window';

import {
SkyPopoverAlignment,
SkyPopoverPlacement,
Expand Down Expand Up @@ -43,8 +39,7 @@ export class SkyPopoverDirective implements OnChanges, OnDestroy {
private idled = new Subject<boolean>();

constructor(
private elementRef: ElementRef,
private windowRef: SkyWindowRefService
private elementRef: ElementRef
) { }

public ngOnChanges(changes: SimpleChanges) {
Expand Down Expand Up @@ -120,7 +115,7 @@ export class SkyPopoverDirective implements OnChanges, OnDestroy {
}
});

Observable
Observable
.fromEvent(element, 'mouseleave')
.takeUntil(this.idled)
.subscribe((event: MouseEvent) => {
Expand All @@ -129,17 +124,15 @@ 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();
Copy link
Member

@Blackbaud-SteveBrush Blackbaud-SteveBrush Aug 16, 2018

Choose a reason for hiding this comment

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

@Blackbaud-TrevorBurch The popover's markForCloseOnMouseLeave() method is no longer being used, but we can't remove it since it would be a breaking change. Do you mind adding a // TODO: ... comment above the method so we don't forget to remove it when we make breaking changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure! Done!

} else {
this.closePopover();
}
}
});
if (this.skyPopover.isOpen) {
this.closePopover();
} 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.closePopover();
});
}
}
});
}
Expand Down