Skip to content

Commit

Permalink
fix(popover): open popover correctly from lifecycle hooks
Browse files Browse the repository at this point in the history
  • Loading branch information
maxokorokov committed Apr 30, 2019
1 parent 80beb3a commit b3f8010
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 3 deletions.
20 changes: 18 additions & 2 deletions src/popover/popover.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {
Injectable,
OnDestroy,
TemplateRef,
ViewContainerRef
ViewContainerRef,
AfterViewInit
} from '@angular/core';

import {Key} from '../util/key';
Expand Down Expand Up @@ -73,7 +74,7 @@ describe('ngb-popover', () => {

beforeEach(() => {
TestBed.configureTestingModule({
declarations: [TestComponent, TestOnPushComponent, DestroyableCmpt],
declarations: [TestComponent, TestOnPushComponent, DestroyableCmpt, TestHooksComponent],
imports: [NgbPopoverModule],
providers: [SpyService]
});
Expand Down Expand Up @@ -349,6 +350,14 @@ describe('ngb-popover', () => {
fixture.detectChanges();
expect(getWindow(fixture.nativeElement)).toBeNull();
});

it('should open popover from hooks', () => {
const fixture = TestBed.createComponent(TestHooksComponent);
fixture.detectChanges();

const popoverWindow = fixture.debugElement.query(By.directive(NgbPopoverWindow));
expect(popoverWindow.nativeElement).toHaveCssClass('popover');
});
});


Expand Down Expand Up @@ -744,3 +753,10 @@ export class DestroyableCmpt implements OnDestroy {

ngOnDestroy(): void { this._spyService.called = true; }
}

@Component({selector: 'test-hooks', template: `<div ngbPopover="popover"></div>`})
export class TestHooksComponent implements AfterViewInit {
@ViewChild(NgbPopover) popover;

ngAfterViewInit() { this.popover.open(); }
}
11 changes: 10 additions & 1 deletion src/popover/popover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,16 @@ export class NgbPopover implements OnInit, OnDestroy, OnChanges {
this._document.querySelector(this.container).appendChild(this._windowRef.location.nativeElement);
}

// apply styling to set basic css-classes on target element, before going for positioning
// We need to detect changes, because we don't know where .open() might be called from.
// Ex. opening popover from one of lifecycle hooks that run after the CD
// (say from ngAfterViewInit) will result in 'ExpressionHasChanged' exception
this._windowRef.changeDetectorRef.detectChanges();

// We need to mark for check, because popover won't work inside the OnPush component.
// Ex. when we use expression like `{{ popover.isOpen() : 'opened' : 'closed' }}`
// inside the template of an OnPush component and we change the popover from
// open -> closed, the expression in question won't be updated unless we explicitly
// mark the parent component to be checked.
this._windowRef.changeDetectorRef.markForCheck();

ngbAutoClose(
Expand Down

0 comments on commit b3f8010

Please sign in to comment.