Merge pull request #135 from ing-bank/fix/popper

Fix update popper config method

packages/overlays/docs/LocalOverlayController.md

@@ -1,6 +1,6 @@
 # LocalOverlayController
 
-This is a base class for different local overlays (e.g. a [tooltip](../../tooltip/), see [Overlay System Implementation](./OverlaySystemImplementation.md) - the ones positioned next to invokers they are related to.
+This is a base class for different local overlays (e.g. a [tooltip](../../tooltip/), see [Overlay System Implementation](./OverlaySystemImplementation.md) - the ones positioned next to invokers they are related to. For more information strictly about the positioning of the content element to the reference element (invoker), please refer to the [positioning documentation](./LocalOverlayPositioning.md)
 You should not use this controller directly unless you want to create a unique type of local overlays which is not supported out of the box.
 
 All supported types of local overlays are described below.

packages/overlays/docs/LocalOverlayPositioning.md

@@ -0,0 +1,2 @@
+# LocalOverlayPositioning
+## Featuring - [Popper.js]()

packages/overlays/src/LocalOverlayController.js

@@ -98,7 +98,7 @@ export class LocalOverlayController {
   async show() {
     this._createOrUpdateOverlay(true, this._prevData);
     /**
-     * Popper is weird about properly positioning the popper element when its is recreated so
+     * Popper is weird about properly positioning the popper element when it is recreated so
      * we just recreate the popper instance to make it behave like it should.
      * Probably related to this issue: https://github.com/FezVrasta/popper.js/issues/796
      * calling just the .update() function on the popper instance sadly does not resolve this.
@@ -128,6 +128,9 @@ export class LocalOverlayController {
   async updatePopperConfig(config = {}) {
     this.__mergePopperConfigs(config);
     await this.__createPopperInstance();
+    if (this.isShown) {
+      this._popper.update();
+    }
   }
 
   _createOrUpdateOverlay(shown = this._prevShown, data = this._prevData) {

packages/overlays/test/LocalOverlayController.test.js

@@ -230,9 +230,9 @@ describe('LocalOverlayController', () => {
       });
 
       await controller.show();
-      // 16px displacement due to default 16px viewport margin both horizontal and vertical
       expect(controller.content.firstElementChild.style.transform).to.equal(
         'translate3d(16px, 16px, 0px)',
+        '16px displacement is expected due to both horizontal and vertical viewport margin',
       );
     });
 
@@ -398,12 +398,18 @@ describe('LocalOverlayController', () => {
 
       await controller.show();
       let contentChild = controller.content.firstElementChild;
-      expect(contentChild.style.transform).to.equal('translate3d(14px, -58px, 0px)');
+      expect(contentChild.style.transform).to.equal(
+        'translate3d(14px, -58px, 0px)',
+        'Popper positioning values',
+      );
 
       controller.hide();
       await controller.show();
       contentChild = controller.content.firstElementChild;
-      expect(contentChild.style.transform).to.equal('translate3d(14px, -58px, 0px)');
+      expect(contentChild.style.transform).to.equal(
+        'translate3d(14px, -58px, 0px)',
+        'Popper positioning values should be identical after hiding and showing',
+      );
     });
 
     it('updates placement properly even during hidden state', async () => {
@@ -429,7 +435,10 @@ describe('LocalOverlayController', () => {
 
       await controller.show();
       let contentChild = controller.content.firstElementChild;
-      expect(contentChild.style.transform).to.equal('translate3d(14px, -58px, 0px)');
+      expect(contentChild.style.transform).to.equal(
+        'translate3d(14px, -58px, 0px)',
+        'Popper positioning values',
+      );
 
       controller.hide();
       await controller.updatePopperConfig({
@@ -443,7 +452,52 @@ describe('LocalOverlayController', () => {
       await controller.show();
       contentChild = controller.content.firstElementChild;
       expect(controller._popper.options.modifiers.offset.offset).to.equal('0, 32px');
-      expect(contentChild.style.transform).to.equal('translate3d(14px, -82px, 0px)');
+      expect(contentChild.style.transform).to.equal(
+        'translate3d(14px, -82px, 0px)',
+        'Popper positioning Y value should be 32 less than previous, due to the added 32px offset',
+      );
+    });
+
+    it('updates positioning correctly during shown state when config gets updated', async () => {
+      const controller = new LocalOverlayController({
+        contentTemplate: () =>
+          html`
+            <p>Content</p>
+          `,
+        invokerTemplate: () => html`
+          <button style="padding: 16px;" @click=${() => controller.show()}>
+            Invoker
+          </button>
+        `,
+        popperConfig: {
+          placement: 'top',
+        },
+      });
+      await fixture(html`
+        <div style="position: absolute; top: 300px; left: 100px;">
+          ${controller.invoker} ${controller.content}
+        </div>
+      `);
+
+      await controller.show();
+      const contentChild = controller.content.firstElementChild;
+      expect(contentChild.style.transform).to.equal(
+        'translate3d(14px, -58px, 0px)',
+        'Popper positioning values',
+      );
+
+      await controller.updatePopperConfig({
+        modifiers: {
+          offset: {
+            enabled: true,
+            offset: '0, 32px',
+          },
+        },
+      });
+      expect(contentChild.style.transform).to.equal(
+        'translate3d(14px, -82px, 0px)',
+        'Popper positioning Y value should be 32 less than previous, due to the added 32px offset',
+      );
     });
   });