Skip to content

Commit 910a1b5

Browse files
Joren BroekemaJoren Broekema
authored andcommitted
fix(overlays): fix update popper config method
1 parent 0b1cb9f commit 910a1b5

File tree

4 files changed

+66
-7
lines changed

4 files changed

+66
-7
lines changed

packages/overlays/docs/LocalOverlayController.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# LocalOverlayController
22

3-
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.
3+
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)
44
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.
55

66
All supported types of local overlays are described below.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# LocalOverlayPositioning
2+
## Featuring - [Popper.js]()

packages/overlays/src/LocalOverlayController.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ export class LocalOverlayController {
9898
async show() {
9999
this._createOrUpdateOverlay(true, this._prevData);
100100
/**
101-
* Popper is weird about properly positioning the popper element when its is recreated so
101+
* Popper is weird about properly positioning the popper element when it is recreated so
102102
* we just recreate the popper instance to make it behave like it should.
103103
* Probably related to this issue: https://github.com/FezVrasta/popper.js/issues/796
104104
* calling just the .update() function on the popper instance sadly does not resolve this.
@@ -128,6 +128,9 @@ export class LocalOverlayController {
128128
async updatePopperConfig(config = {}) {
129129
this.__mergePopperConfigs(config);
130130
await this.__createPopperInstance();
131+
if (this.isShown) {
132+
this._popper.update();
133+
}
131134
}
132135

133136
_createOrUpdateOverlay(shown = this._prevShown, data = this._prevData) {

packages/overlays/test/LocalOverlayController.test.js

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -230,9 +230,9 @@ describe('LocalOverlayController', () => {
230230
});
231231

232232
await controller.show();
233-
// 16px displacement due to default 16px viewport margin both horizontal and vertical
234233
expect(controller.content.firstElementChild.style.transform).to.equal(
235234
'translate3d(16px, 16px, 0px)',
235+
'16px displacement is expected due to both horizontal and vertical viewport margin',
236236
);
237237
});
238238

@@ -398,12 +398,18 @@ describe('LocalOverlayController', () => {
398398

399399
await controller.show();
400400
let contentChild = controller.content.firstElementChild;
401-
expect(contentChild.style.transform).to.equal('translate3d(14px, -58px, 0px)');
401+
expect(contentChild.style.transform).to.equal(
402+
'translate3d(14px, -58px, 0px)',
403+
'Popper positioning values',
404+
);
402405

403406
controller.hide();
404407
await controller.show();
405408
contentChild = controller.content.firstElementChild;
406-
expect(contentChild.style.transform).to.equal('translate3d(14px, -58px, 0px)');
409+
expect(contentChild.style.transform).to.equal(
410+
'translate3d(14px, -58px, 0px)',
411+
'Popper positioning values should be identical after hiding and showing',
412+
);
407413
});
408414

409415
it('updates placement properly even during hidden state', async () => {
@@ -429,7 +435,10 @@ describe('LocalOverlayController', () => {
429435

430436
await controller.show();
431437
let contentChild = controller.content.firstElementChild;
432-
expect(contentChild.style.transform).to.equal('translate3d(14px, -58px, 0px)');
438+
expect(contentChild.style.transform).to.equal(
439+
'translate3d(14px, -58px, 0px)',
440+
'Popper positioning values',
441+
);
433442

434443
controller.hide();
435444
await controller.updatePopperConfig({
@@ -443,7 +452,52 @@ describe('LocalOverlayController', () => {
443452
await controller.show();
444453
contentChild = controller.content.firstElementChild;
445454
expect(controller._popper.options.modifiers.offset.offset).to.equal('0, 32px');
446-
expect(contentChild.style.transform).to.equal('translate3d(14px, -82px, 0px)');
455+
expect(contentChild.style.transform).to.equal(
456+
'translate3d(14px, -82px, 0px)',
457+
'Popper positioning Y value should be 32 less than previous, due to the added 32px offset',
458+
);
459+
});
460+
461+
it('updates positioning correctly during shown state when config gets updated', async () => {
462+
const controller = new LocalOverlayController({
463+
contentTemplate: () =>
464+
html`
465+
<p>Content</p>
466+
`,
467+
invokerTemplate: () => html`
468+
<button style="padding: 16px;" @click=${() => controller.show()}>
469+
Invoker
470+
</button>
471+
`,
472+
popperConfig: {
473+
placement: 'top',
474+
},
475+
});
476+
await fixture(html`
477+
<div style="position: absolute; top: 300px; left: 100px;">
478+
${controller.invoker} ${controller.content}
479+
</div>
480+
`);
481+
482+
await controller.show();
483+
const contentChild = controller.content.firstElementChild;
484+
expect(contentChild.style.transform).to.equal(
485+
'translate3d(14px, -58px, 0px)',
486+
'Popper positioning values',
487+
);
488+
489+
await controller.updatePopperConfig({
490+
modifiers: {
491+
offset: {
492+
enabled: true,
493+
offset: '0, 32px',
494+
},
495+
},
496+
});
497+
expect(contentChild.style.transform).to.equal(
498+
'translate3d(14px, -82px, 0px)',
499+
'Popper positioning Y value should be 32 less than previous, due to the added 32px offset',
500+
);
447501
});
448502
});
449503

0 commit comments

Comments
 (0)