feat(positioning): use popper 2 for positioning (#4129)

BREAKING CHANGES: ng-bootstrap now has a dependency on `@popper/core`. All positioning logic for datepicker, dropdown, popover, tooltip and typeahead is outsourced to popper.js
ng-bootstrap · Oct 26, 2021 · 5842eb5 · 5842eb5
1 parent 9598506
commit 5842eb5
Show file tree

Hide file tree

Showing 19 changed files with 811 additions and 1,075 deletions.
diff --git a/demo/src/style/app.scss b/demo/src/style/app.scss
@@ -1,17 +1,5 @@
 // styles in src/style directory are applied to the whole page
 
-// Temporary fix, as this classes are required for the positioning
-// They existed in bs4, and we must remove them after popper implementation
-.dropdown-menu {
-  &[x-placement^="top"],
-  &[x-placement^="end"],
-  &[x-placement^="bottom"],
-  &[x-placement^="start"] {
-    right: auto;
-    bottom: auto;
-  }
-}
-
 header.navbar {
   background: linear-gradient(135deg, #0143a3, #0273d4);
 }

diff --git a/e2e-app/setup.e2e-spec.ts b/e2e-app/setup.e2e-spec.ts
@@ -1,6 +1,5 @@
 import * as fs from 'fs';
 import * as path from 'path';
-import {ConsoleMessage} from 'playwright';
 import {browserName, test, launchOptions} from './playwright.conf';
 
 beforeAll(async() => {
@@ -13,15 +12,15 @@ beforeAll(async() => {
     console.error('Unable to setup a new page with playwright', e);
   }
 
+  // Need to wait to avoid connection failures between Playwright and the browser.
+  // To remove once Playwright test runner will be used.
+  await test.page.waitForTimeout(500);
+
   // Listen for all console events and handle errors
   test.page.on('console', async msg => {
     const type = msg.type();
     if (type === 'error' || type === 'warning') {
-      const output = ['Unexpected console error:'];
-      for (let m of msg.args()) {
-        output.push(await m.jsonValue());
-      }
-      fail(output.join('\n'));
+      fail(msg.text());
     }
   });
 

diff --git a/e2e-app/src/style/app.scss b/e2e-app/src/style/app.scss
@@ -1,13 +1 @@
 /* You can add global styles to this file, and also import other style files */
-
-// Temporary fix, as this classes are required for the positioning
-// They existed in bs4, and we must remove them after popper implementation
-.dropdown-menu {
-    &[x-placement^="top"],
-    &[x-placement^="end"],
-    &[x-placement^="bottom"],
-    &[x-placement^="start"] {
-      right: auto;
-      bottom: auto;
-    }
-  }
diff --git a/package.json b/package.json
@@ -91,6 +91,7 @@
     "@commitlint/config-angular": "12.1.4",
     "@nguniversal/express-engine": "~12.0.2",
     "@schematics/angular": "~12.0.5",
+    "@popperjs/core": "^2.10.2",
     "@types/express": "^4.16.1",
     "@types/fs-extra": "^9.0.3",
     "@types/glob": "^7.1.1",
@@ -99,7 +100,7 @@
     "@types/marked": "^1.1.0",
     "@types/node": "^12.11.1",
     "@types/prismjs": "1.16.2",
-    "bootstrap": "5.0.2",
+    "bootstrap": "5.1.2",
     "clang-format": "1.0.35",
     "concurrently": "^5.3.0",
     "conventional-changelog-cli": "^2.0.12",
@@ -126,7 +127,7 @@
     "ng-packagr": "^12.0.6",
     "ngx-build-plus": "^12.0.1",
     "nyc": "14.1.1",
-    "playwright": "^1.9.1",
+    "playwright": "^1.16.0",
     "prismjs": "1.22.0",
     "rimraf": "^3.0.2",
     "rxjs": "~6.5.5",
@@ -141,4 +142,4 @@
     "yargs": "^16.2.0",
     "zone.js": "~0.11.4"
   }
-}
+}
diff --git a/src/datepicker/datepicker-input.spec.ts b/src/datepicker/datepicker-input.spec.ts
@@ -10,7 +10,6 @@ import {NgbInputDatepicker} from './datepicker-input';
 import {NgbDatepicker} from './datepicker';
 import {NgbDateStruct} from './ngb-date-struct';
 import {NgbDate} from './ngb-date';
-import {positionService} from 'src/util/positioning';
 import {NgbInputDatepickerConfig} from './datepicker-input-config';
 
 const createTestCmpt = (html: string) =>
@@ -945,65 +944,11 @@ describe('NgbInputDatepicker', () => {
 
   describe('positionTarget', () => {
 
-    let positionElementsSpy: jasmine.Spy;
-
-    beforeEach(() => { positionElementsSpy = spyOn(positionService, 'position').and.callThrough(); });
-
-    it('should position popup by input if no target provided (default)', () => {
-      const fixture = createTestCmpt(`
-          <input ngbDatepicker #d="ngbDatepicker">
-          <button (click)="open(d)">Open</button>
-      `);
-      const input = fixture.nativeElement.querySelector('input');
-
-      // open date-picker
-      const button = fixture.nativeElement.querySelector('button');
-      button.click();
-      fixture.detectChanges();
-
-      expect(positionElementsSpy).toHaveBeenCalled();
-      expect(positionElementsSpy.calls.argsFor(0)[0]).toBe(input);
-    });
-
-    it('should position popup by html element', () => {
-      const fixture = createTestCmpt(`
-          <input ngbDatepicker #d="ngbDatepicker" [positionTarget]="myButton">
-          <button #myButton (click)="open(d)">Open</button>
-      `);
-
-      // open date-picker
-      const button = fixture.nativeElement.querySelector('button');
-      button.click();
-      fixture.detectChanges();
-
-      expect(positionElementsSpy).toHaveBeenCalled();
-      expect(positionElementsSpy.calls.argsFor(0)[0]).toBe(button);
-    });
-
-    it('should position popup by css selector', () => {
-      const selector = '#myButton';
-      const fixture = createTestCmpt(`
-          <input ngbDatepicker #d="ngbDatepicker" positionTarget="${selector}">
-          <button id="myButton" (click)="open(d)">Open</button>
-      `);
-
-      // open date-picker
-      const button = fixture.nativeElement.querySelector(selector);
-      button.click();
-      fixture.detectChanges();
-
-      expect(positionElementsSpy).toHaveBeenCalled();
-      expect(positionElementsSpy.calls.argsFor(0)[0]).toBe(button);
-    });
-
     it('should throw error if target element does not exists', fakeAsync(() => {
          const fixture = createTestCmpt(`<input ngbDatepicker #d="ngbDatepicker" positionTarget="#nobody">`);
          const dpInput = fixture.debugElement.query(By.directive(NgbInputDatepicker)).injector.get(NgbInputDatepicker);
 
-         dpInput.open();
-         fixture.detectChanges();
-
-         expect(() => tick())
+         expect(() => dpInput.open())
              .toThrowError('ngbDatepicker could not find element declared in [positionTarget] to position against.');
        }));
   });

diff --git a/src/datepicker/datepicker-input.ts b/src/datepicker/datepicker-input.ts
@@ -29,7 +29,7 @@ import {
 
 import {ngbAutoClose} from '../util/autoclose';
 import {ngbFocusTrap} from '../util/focus-trap';
-import {PlacementArray, positionElements} from '../util/positioning';
+import {PlacementArray, ngbPositioning} from '../util/positioning';
 
 import {NgbDateAdapter} from './adapters/ngb-date-adapter';
 import {NgbDatepicker, NgbDatepickerNavigateEvent} from './datepicker';
@@ -41,6 +41,7 @@ import {NgbDateStruct} from './ngb-date-struct';
 import {NgbInputDatepickerConfig} from './datepicker-input-config';
 import {NgbDatepickerConfig} from './datepicker-config';
 import {isString} from '../util/util';
+import {take} from 'rxjs/operators';
 
 /**
  * A directive that allows to stick a datepicker popup to an input field.
@@ -77,6 +78,7 @@ export class NgbInputDatepicker implements OnChanges,
   private _model: NgbDate | null = null;
   private _inputValue: string;
   private _zoneSubscription: any;
+  private _positioning = ngbPositioning();
 
   /**
    * Indicates whether the datepicker popup should be closed automatically after date selection / outside click or not.
@@ -181,13 +183,15 @@ export class NgbInputDatepicker implements OnChanges,
   /**
    * The preferred placement of the datepicker popup.
    *
-   * Possible values are `"top"`, `"top-start"`, `"top-end"`, `"bottom"`, `"bottom-start"`,
-   * `"bottom-end"`, `"start"`, `"start-top"`, `"start-bottom"`, `"end"`, `"end-top"`,
-   * `"end-bottom"`
+   * Possible values are
+   * `"top"`, `"top-start"`, `"top-left"`, `"top-end"`, `"top-right"`,
+   * `"bottom"`, `"bottom-start"`, `"bottom-left"`, `"bottom-end"`, `"bottom-right"`,
+   * `"start"`, `left`, `"start-top"`, `"left-top"`, `"start-bottom"`, `"left-bottom"`,
+   * `"end"`, `"right"`, `"end-top"`, `"right-top"`, `"end-bottom"`, `"right-bottom"`
    *
    * Accepts an array of strings or a string with space separated possible values.
    *
-   * The default order of preference is `"bottom-left bottom-right top-left top-right"`
+   * The default order of preference is `"bottom-start bottom-end top-start top-end"`
    *
    * Please see the [positioning overview](#/positioning) for more details.
    */
@@ -292,7 +296,7 @@ export class NgbInputDatepicker implements OnChanges,
       @Inject(DOCUMENT) private _document: any, private _changeDetector: ChangeDetectorRef,
       config: NgbInputDatepickerConfig) {
     ['autoClose', 'container', 'positionTarget', 'placement'].forEach(input => this[input] = config[input]);
-    this._zoneSubscription = _ngZone.onStable.subscribe(() => this._updatePopupPosition());
+    this._zoneSubscription = _ngZone.onStable.subscribe(() => this._positioning.update());
   }
 
   registerOnChange(fn: (value: any) => any): void { this._onChange = fn; }
@@ -382,6 +386,30 @@ export class NgbInputDatepicker implements OnChanges,
       ngbFocusTrap(this._ngZone, this._cRef.location.nativeElement, this.closed, true);
       this._cRef.instance.focus();
 
+      let hostElement: HTMLElement;
+      if (isString(this.positionTarget)) {
+        hostElement = this._document.querySelector(this.positionTarget);
+      } else if (this.positionTarget instanceof HTMLElement) {
+        hostElement = this.positionTarget;
+      } else {
+        hostElement = this._elRef.nativeElement;
+      }
+
+      this._ngZone.onStable.pipe(take(1)).subscribe(() => {
+        if (this._cRef) {
+          this._positioning.createPopper({
+            hostElement,
+            targetElement: this._cRef.location.nativeElement,
+            placement: this.placement,
+            appendToBody: this.container === 'body',
+          });
+        }
+      });
+
+      if (this.positionTarget && !hostElement) {
+        throw new Error('ngbDatepicker could not find element declared in [positionTarget] to position against.');
+      }
+
       ngbAutoClose(
           this._ngZone, this._document, this.autoClose, () => this.close(), this.closed, [],
           [this._elRef.nativeElement, this._cRef.location.nativeElement]);
@@ -412,6 +440,8 @@ export class NgbInputDatepicker implements OnChanges,
       } else {
         this._document.body.focus();
       }
+
+      this._positioning.destroy();
     }
   }
 
@@ -528,25 +558,4 @@ export class NgbInputDatepicker implements OnChanges,
     const ngbDate = date ? new NgbDate(date.year, date.month, date.day) : null;
     return this._calendar.isValid(ngbDate) ? ngbDate : null;
   }
-
-  private _updatePopupPosition() {
-    if (!this._cRef) {
-      return;
-    }
-
-    let hostElement: HTMLElement;
-    if (isString(this.positionTarget)) {
-      hostElement = this._document.querySelector(this.positionTarget);
-    } else if (this.positionTarget instanceof HTMLElement) {
-      hostElement = this.positionTarget;
-    } else {
-      hostElement = this._elRef.nativeElement;
-    }
-
-    if (this.positionTarget && !hostElement) {
-      throw new Error('ngbDatepicker could not find element declared in [positionTarget] to position against.');
-    }
-
-    positionElements(hostElement, this._cRef.location.nativeElement, this.placement, this.container === 'body');
-  }
 }