Skip to content

Commit 17738c2

Browse files
refactor!: close popover without trigger on Esc and outside click (#8409) (#8666)
Co-authored-by: Serhii Kulykov <[email protected]>
1 parent e0ea8b9 commit 17738c2

File tree

3 files changed

+17
-27
lines changed

3 files changed

+17
-27
lines changed

packages/popover/src/vaadin-popover.d.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,8 @@ declare class Popover extends PopoverPositionMixin(
221221
* - outside click (unless `noCloseOnOutsideClick` property is true)
222222
*
223223
* When setting `trigger` property to `null`, `undefined` or empty array, the popover
224-
* can be only opened or closed programmatically by changing `opened` property.
224+
* can be only opened programmatically by changing `opened` property. Note, closing
225+
* on Escape press or outside click is still allowed unless explicitly disabled.
225226
*/
226227
trigger: PopoverTrigger[] | null | undefined;
227228

packages/popover/src/vaadin-popover.js

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,8 @@ class Popover extends PopoverPositionMixin(
372372
* - outside click (unless `noCloseOnOutsideClick` property is true)
373373
*
374374
* When setting `trigger` property to `null`, `undefined` or empty array, the popover
375-
* can be only opened or closed programmatically by changing `opened` property.
375+
* can be only opened programmatically by changing `opened` property. Note, closing
376+
* on Escape press or outside click is still allowed unless explicitly disabled.
376377
*/
377378
trigger: {
378379
type: Array,
@@ -611,7 +612,6 @@ class Popover extends PopoverPositionMixin(
611612
__onGlobalClick(event) {
612613
if (
613614
this.opened &&
614-
!this.__isManual &&
615615
!this.modal &&
616616
!event.composedPath().some((el) => el === this._overlayElement || el === this.target) &&
617617
!this.noCloseOnOutsideClick &&
@@ -646,13 +646,7 @@ class Popover extends PopoverPositionMixin(
646646
return;
647647
}
648648

649-
if (
650-
event.key === 'Escape' &&
651-
!this.noCloseOnEsc &&
652-
this.opened &&
653-
!this.__isManual &&
654-
isLastOverlay(this._overlayElement)
655-
) {
649+
if (event.key === 'Escape' && !this.noCloseOnEsc && this.opened && isLastOverlay(this._overlayElement)) {
656650
// Prevent closing parent overlay (e.g. dialog)
657651
event.stopPropagation();
658652
this._openedStateController.close(true);
@@ -939,7 +933,7 @@ class Popover extends PopoverPositionMixin(
939933
* @private
940934
*/
941935
__onEscapePress(e) {
942-
if (this.noCloseOnEsc || this.__isManual) {
936+
if (this.noCloseOnEsc) {
943937
e.preventDefault();
944938
}
945939
}
@@ -949,7 +943,7 @@ class Popover extends PopoverPositionMixin(
949943
* @private
950944
*/
951945
__onOutsideClick(e) {
952-
if (this.noCloseOnOutsideClick || this.__isManual) {
946+
if (this.noCloseOnOutsideClick) {
953947
e.preventDefault();
954948
}
955949
}
@@ -959,11 +953,6 @@ class Popover extends PopoverPositionMixin(
959953
return Array.isArray(this.trigger) && this.trigger.includes(trigger);
960954
}
961955

962-
/** @private */
963-
get __isManual() {
964-
return this.trigger == null || (Array.isArray(this.trigger) && this.trigger.length === 0);
965-
}
966-
967956
/** @private */
968957
__updateDimension(overlay, dimension, value) {
969958
const prop = `--_vaadin-popover-content-${dimension}`;

packages/popover/test/trigger.test.js

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -492,51 +492,51 @@ describe('trigger', () => {
492492
expect(overlay.opened).to.be.true;
493493
});
494494

495-
it(`should not close on global Escape press with trigger set to ${trigger}`, async () => {
495+
it(`should close on global Escape press with trigger set to ${trigger}`, async () => {
496496
popover.opened = true;
497497
await nextRender();
498498

499499
esc(document.body);
500500
await nextUpdate(popover);
501-
expect(overlay.opened).to.be.true;
501+
expect(overlay.opened).to.be.false;
502502
});
503503

504-
it(`should not close on target Escape press with trigger set to ${trigger}`, async () => {
504+
it(`should close on target Escape press with trigger set to ${trigger}`, async () => {
505505
popover.opened = true;
506506
await nextRender();
507507

508508
esc(target);
509509
await nextUpdate(popover);
510-
expect(overlay.opened).to.be.true;
510+
expect(overlay.opened).to.be.false;
511511
});
512512

513-
it(`should not close on global Escape press when modal with trigger set to ${trigger}`, async () => {
513+
it(`should close on global Escape press when modal with trigger set to ${trigger}`, async () => {
514514
popover.modal = true;
515515
popover.opened = true;
516516
await nextRender();
517517

518518
esc(document.body);
519519
await nextUpdate(popover);
520-
expect(overlay.opened).to.be.true;
520+
expect(overlay.opened).to.be.false;
521521
});
522522

523-
it(`should not close on outside click when not modal with trigger set to ${trigger}`, async () => {
523+
it(`should close on outside click when not modal with trigger set to ${trigger}`, async () => {
524524
popover.opened = true;
525525
await nextRender();
526526

527527
outsideClick();
528528
await nextUpdate(popover);
529-
expect(overlay.opened).to.be.true;
529+
expect(overlay.opened).to.be.false;
530530
});
531531

532-
it(`should not close on outside click when modal with trigger set to ${trigger}`, async () => {
532+
it(`should close on outside click when modal with trigger set to ${trigger}`, async () => {
533533
popover.modal = true;
534534
popover.opened = true;
535535
await nextRender();
536536

537537
outsideClick();
538538
await nextUpdate(popover);
539-
expect(overlay.opened).to.be.true;
539+
expect(overlay.opened).to.be.false;
540540
});
541541

542542
it(`should close when setting opened to false with trigger set to ${trigger}`, async () => {

0 commit comments

Comments
 (0)