Skip to content

Commit

Permalink
fix(dropdown): allow template directive on the menu (ng-bootstrap#3263)
Browse files Browse the repository at this point in the history
  • Loading branch information
@fbasso
fbasso authored and Benoit Charbonnier committed Jul 15, 2019
1 parent ffcdad4 commit 8f73899
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 86 deletions.
38 changes: 26 additions & 12 deletions e2e-app/src/app/dropdown/position/dropdown-position.component.html
View file
Expand Up @@ -2,23 +2,37 @@ <h3>Dropdown positioning tests</h3>

<form id="default">
<div *ngIf="isInDom" class="form-group form-inline d-inline-block mr-3">
<div id="dropdown" ngbDropdown class="d-inline-block mr-1" [autoClose]="false" [container]="container" [placement]="placement">
<div id="dropdown" ngbDropdown class="d-inline-block mr-1 mb-1" [autoClose]="false" [container]="container" [placement]="placement">
<button class="btn btn-outline-secondary" ngbDropdownToggle>Toggle dropdown</button>
<div ngbDropdownMenu id="dropdownmenu">
<div ngbDropdownMenu id="dropdownMenu">
<button class="dropdown-item">Item</button>
</div>
</div>
<br />
<div id="dropdownWithTemplate" ngbDropdown class="d-inline-block" [autoClose]="false" [container]="container" [placement]="placement">
<button class="btn btn-outline-secondary" ngbDropdownToggle>Dropdown with template</button>
<div ngbDropdownMenu id="dropdownWithTemplateMenu" *ngIf="true">
<button ngbDropdownItem>Action - 1</button>
<button ngbDropdownItem>Another Action</button>
<button ngbDropdownItem>Something else is here</button>
</div>
</div>
</div>

<div class="d-inline-block ml-3">
<button class="btn btn-outline-secondary mr-1" id="isInDom-true" (click)="isInDom = true">Add in dom</button>
<button class="btn btn-outline-secondary mr-1" id="isInDom-false" (click)="isInDom = false">Remove from dom</button>
<div class="mb-1">
<button class="btn btn-outline-secondary mr-1" id="isInDom-true" (click)="isInDom = true">Add in dom</button>
<button class="btn btn-outline-secondary mr-1" id="isInDom-false" (click)="isInDom = false">Remove from dom</button>
<button class="btn btn-outline-secondary mr-1 ml-2" id="container-body" (click)="toggleContainer('body')">Append to body</button>
<button class="btn btn-outline-secondary" id="container-null" (click)="toggleContainer(null)">Append to widget</button>
</div>
<div>
<button class="btn btn-outline-secondary mr-1 ml-2" id="placement-top-left" (click)="togglePlacement('top-left')">On top-left</button>
<button class="btn btn-outline-secondary mr-1" id="placement-bottom-left" (click)="togglePlacement('bottom-left')">On bottom-left</button>
<button class="btn btn-outline-secondary mr-1 ml-2" id="placement-top-right" (click)="togglePlacement('top-right')">On top-right</button>
<button class="btn btn-outline-secondary mr-1" id="placement-bottom-right" (click)="togglePlacement('bottom-right')">On bottom-right</button>
</div>
</div>

<button class="btn btn-outline-secondary mr-1 ml-2" id="container-body" (click)="toggleContainer('body')">Append to body</button>
<button class="btn btn-outline-secondary" id="container-null" (click)="toggleContainer(null)">Append to widget</button>
</form>

<button class="btn btn-outline-secondary mr-1 ml-2" id="placement-top-left" (click)="togglePlacement('top-left')">On top-left</button>
<button class="btn btn-outline-secondary mr-1" id="placement-bottom-left" (click)="togglePlacement('bottom-left')">On bottom-left</button>
<button class="btn btn-outline-secondary mr-1 ml-2" id="placement-top-right" (click)="togglePlacement('top-right')">On top-right</button>
<button class="btn btn-outline-secondary mr-1" id="placement-bottom-right" (click)="togglePlacement('bottom-right')">On bottom-right</button>
</div>
</form>
112 changes: 57 additions & 55 deletions e2e-app/src/app/dropdown/position/dropdown-position.e2e-spec.ts
View file
Expand Up @@ -10,82 +10,84 @@ const roundLocation = function(location) {
return location;
};

['#dropdown', '#dropdownWithTemplate'].forEach((selector) => {
describe(`Dropdown Position for id ${selector}`, () => {
let dropdownPositionPage: DropdownPositionPage;
let dropdownPage: DropdownPage;

describe('Dropdown Position', () => {
let dropdownPositionPage: DropdownPositionPage;
let dropdownPage: DropdownPage;

const expectSamePositions = async(placement) => {

const expectSamePositions = async(placement) => {
// Setup start conditions
await dropdownPositionPage.toggleContainer(null);
await dropdownPositionPage.togglePlacement(placement);

// Setup start conditions
await dropdownPositionPage.toggleContainer(null);
await dropdownPositionPage.togglePlacement(placement);
const dropdownMenu = dropdownPage.getDropdownMenu(`${selector}Menu`);
expect(await dropdownPage.getDropdownMenuParent(dropdownMenu).getAttribute('ngbdropdown'))
.toBe('', 'The dropdown menu should be appended to the widget');

const dropdownMenu = dropdownPage.getDropdownMenu('#dropdownmenu');
expect(await dropdownPage.getDropdownMenuParent(dropdownMenu).getAttribute('ngbdropdown'))
.toBe('', 'The dropdown menu should be appended to the widget');
// Get position in widget
const widgetLocation = roundLocation(await dropdownMenu.getLocation());

// Get position in widget
const widgetLocation = roundLocation(await dropdownMenu.getLocation());
// Compare position to body
await dropdownPositionPage.toggleContainer('body');
expect(await dropdownPage.getDropdownMenuParent(dropdownMenu).element(by.xpath('..')).getTagName())
.toBe('body', 'The dropdown menu should be appended to the body');

// Compare position to body
await dropdownPositionPage.toggleContainer('body');
expect(await dropdownPage.getDropdownMenuParent(dropdownMenu).element(by.xpath('..')).getTagName())
.toBe('body', 'The dropdown menu should be appended to the body');
const bodyLocation = roundLocation(await dropdownMenu.getLocation());
expect(bodyLocation)
.toEqual(widgetLocation, `Positions should give the same results when placed on ${placement}`);

const bodyLocation = roundLocation(await dropdownMenu.getLocation());
expect(bodyLocation).toEqual(widgetLocation, `Positions should give the same results when placed on ${placement}`);
// Reset
await dropdownPositionPage.toggleContainer(null);

// Reset
await dropdownPositionPage.toggleContainer(null);
};

};
beforeAll(() => {
dropdownPositionPage = new DropdownPositionPage();
dropdownPage = new DropdownPage();
});

beforeAll(() => {
dropdownPositionPage = new DropdownPositionPage();
dropdownPage = new DropdownPage();
});
beforeEach(async() => await openUrl('dropdown/position'));

beforeEach(async() => await openUrl('dropdown/position'));
it(`should keep the same position when appended to widget or body`, async() => {

it(`should keep the same position when appended to widget or body`, async() => {
const dropdown = dropdownPage.getDropdown(selector);
await dropdownPage.open(dropdown);

const dropdown = dropdownPage.getDropdown('#dropdown');
await dropdownPage.open(dropdown);
await expectSamePositions('bottom-left');
await expectSamePositions('top-left');
await expectSamePositions('bottom-right');
await expectSamePositions('top-right');

await expectSamePositions('bottom-left');
await expectSamePositions('top-left');
await expectSamePositions('bottom-right');
await expectSamePositions('top-right');
});

});
it(`should be removed on destroy`, async() => {
const dropdown = dropdownPage.getDropdown(selector);
await dropdownPage.open(dropdown);
await dropdownPositionPage.removeFromDom();
expect(await dropdownPage.getDropdownMenu(`${selector}Menu`).isPresent())
.toBeFalsy(`Dropdown shouldn't be in the dom`);
});

it(`should be removed on destroy`, async() => {
const dropdown = dropdownPage.getDropdown('#dropdown');
await dropdownPage.open(dropdown);
await dropdownPositionPage.removeFromDom();
expect(await dropdownPage.getDropdownMenu('#dropdownmenu').isPresent())
.toBeFalsy(`Dropdown shouldn't be in the dom`);
});
it(`should have the body container added and removed`, async() => {
const dropdown = dropdownPage.getDropdown(selector);
await dropdownPositionPage.toggleContainer('body');
await dropdownPage.open(dropdown);

it(`should have the body container added and removed`, async() => {
const dropdown = dropdownPage.getDropdown('#dropdown');
await dropdownPositionPage.toggleContainer('body');
await dropdownPage.open(dropdown);
await dropdownPositionPage.togglePlacement('bottom-left');
expect(await dropdownPage.getBodyContainers().count())
.toBe(1, `Dropdown menu container should be found on the body`);

await dropdownPositionPage.togglePlacement('bottom-left');
expect(await dropdownPage.getBodyContainers().count())
.toBe(1, `Dropdown menu container should be found on the body`);
await dropdownPositionPage.togglePlacement('top-left');
expect(await dropdownPage.getBodyContainers().count())
.toBe(1, `Dropdown menu container should be found on the body`);

await dropdownPositionPage.togglePlacement('top-left');
expect(await dropdownPage.getBodyContainers().count())
.toBe(1, `Dropdown menu container should be found on the body`);
await dropdownPositionPage.toggleContainer(null);
expect(await dropdownPage.getBodyContainers().count())
.toBe(0, `Dropdown menu container shouldn't be found on the body`);

await dropdownPositionPage.toggleContainer(null);
expect(await dropdownPage.getBodyContainers().count())
.toBe(0, `Dropdown menu container shouldn't be found on the body`);
});

});

});
19 changes: 19 additions & 0 deletions src/dropdown/dropdown.spec.ts
View file
Expand Up @@ -113,6 +113,25 @@ describe('ngb-dropdown', () => {
const fixture = createTestComponent(html);
const compiled = fixture.nativeElement;

fixture.detectChanges();
expect(getMenuEl(compiled).getAttribute('x-placement')).toBe('bottom-right');
});

it('should have x-placement attribute reflecting placement with a template', () => {

const html = `
<div ngbDropdown placement="bottom-right">
<button ngbDropdownAnchor></button>
<div *ngIf="true" ngbDropdownMenu>
<a class="dropdown-item">dropDown item</a>
<a class="dropdown-item">dropDown item</a>
</div>
</div>`;

const fixture = createTestComponent(html);
const compiled = fixture.nativeElement;

fixture.detectChanges();
expect(getMenuEl(compiled).getAttribute('x-placement')).toBe('bottom-right');
});

Expand Down
45 changes: 26 additions & 19 deletions src/dropdown/dropdown.ts
View file
Expand Up @@ -9,8 +9,8 @@ import {
Inject,
Input,
NgZone,
AfterContentInit,
OnDestroy,
OnInit,
Output,
QueryList,
Renderer2,
Expand All @@ -19,6 +19,7 @@ import {
} from '@angular/core';
import {DOCUMENT} from '@angular/common';
import {Subject, Subscription} from 'rxjs';
import {take} from 'rxjs/operators';

import {Placement, PlacementArray, positionElements} from '../util/positioning';
import {ngbAutoClose} from '../util/autoclose';
Expand Down Expand Up @@ -128,15 +129,14 @@ export class NgbDropdownToggle extends NgbDropdownAnchor {
* A directive that provides contextual overlays for displaying lists of links and more.
*/
@Directive({selector: '[ngbDropdown]', exportAs: 'ngbDropdown', host: {'[class.show]': 'isOpen()'}})
export class NgbDropdown implements OnInit, OnDestroy {
export class NgbDropdown implements AfterContentInit, OnDestroy {
private _closed$ = new Subject<void>();
private _zoneSubscription: Subscription;
private _bodyContainer: HTMLElement;

@ContentChild(NgbDropdownMenu, {static: true}) private _menu: NgbDropdownMenu;
@ContentChild(NgbDropdownMenu, {read: ElementRef, static: true}) private _menuElement: ElementRef;

@ContentChild(NgbDropdownAnchor, {static: true}) private _anchor: NgbDropdownAnchor;
@ContentChild(NgbDropdownMenu, {static: false}) private _menu: NgbDropdownMenu;
@ContentChild(NgbDropdownMenu, {read: ElementRef, static: false}) private _menuElement: ElementRef;
@ContentChild(NgbDropdownAnchor, {static: false}) private _anchor: NgbDropdownAnchor;

/**
* Indicates whether the dropdown should be closed when clicking one of dropdown items or pressing ESC.
Expand Down Expand Up @@ -205,11 +205,13 @@ export class NgbDropdown implements OnInit, OnDestroy {
this._zoneSubscription = _ngZone.onStable.subscribe(() => { this._positionMenu(); });
}

ngOnInit() {
this._applyPlacementClasses();
if (this._open) {
this._setCloseHandlers();
}
ngAfterContentInit() {
this._ngZone.onStable.pipe(take(1)).subscribe(() => {
this._applyPlacementClasses();
if (this._open) {
this._setCloseHandlers();
}
});
}

ngOnChanges(changes: SimpleChanges) {
Expand Down Expand Up @@ -240,9 +242,10 @@ export class NgbDropdown implements OnInit, OnDestroy {
}

private _setCloseHandlers() {
const anchor = this._anchor;
ngbAutoClose(
this._ngZone, this._document, this.autoClose, () => this.close(), this._closed$,
this._menu ? [this._menuElement.nativeElement] : [], this._anchor ? [this._anchor.getNativeElement()] : [],
this._menu ? [this._menuElement.nativeElement] : [], anchor ? [anchor.getNativeElement()] : [],
'.dropdown-item,.dropdown-divider');
}

Expand Down Expand Up @@ -341,14 +344,16 @@ export class NgbDropdown implements OnInit, OnDestroy {
}

private _getMenuElements(): HTMLElement[] {
if (this._menu == null) {
const menu = this._menu;
if (menu == null) {
return [];
}
return this._menu.menuItems.filter(item => !item.disabled).map(item => item.elementRef.nativeElement);
return menu.menuItems.filter(item => !item.disabled).map(item => item.elementRef.nativeElement);
}

private _positionMenu() {
if (this.isOpen() && this._menu) {
const menu = this._menu;
if (this.isOpen() && menu) {
this._applyPlacementClasses(
this.display === 'dynamic' ?
positionElements(
Expand All @@ -364,9 +369,10 @@ export class NgbDropdown implements OnInit, OnDestroy {

private _resetContainer() {
const renderer = this._renderer;
if (this._menuElement) {
const menuElement = this._menuElement;
if (menuElement) {
const dropdownElement = this._elementRef.nativeElement;
const dropdownMenuElement = this._menuElement.nativeElement;
const dropdownMenuElement = menuElement.nativeElement;

renderer.appendChild(dropdownElement, dropdownMenuElement);
renderer.removeStyle(dropdownMenuElement, 'position');
Expand Down Expand Up @@ -396,7 +402,8 @@ export class NgbDropdown implements OnInit, OnDestroy {
}

private _applyPlacementClasses(placement?: Placement) {
if (this._menu) {
const menu = this._menu;
if (menu) {
if (!placement) {
placement = this._getFirstPlacement(this.placement);
}
Expand All @@ -407,7 +414,7 @@ export class NgbDropdown implements OnInit, OnDestroy {
// remove the current placement classes
renderer.removeClass(dropdownElement, 'dropup');
renderer.removeClass(dropdownElement, 'dropdown');
this._menu.placement = this.display === 'static' ? null : placement;
menu.placement = this.display === 'static' ? null : placement;

/*
* apply the new placement
Expand Down

0 comments on commit 8f73899

Please sign in to comment.