Skip to content

Commit

Permalink
feat: add properties to control popover closing behavior (#7414)
Browse files Browse the repository at this point in the history
  • Loading branch information
web-padawan authored May 16, 2024
1 parent 0e6fb45 commit 645f463
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 5 deletions.
2 changes: 1 addition & 1 deletion packages/popover/src/vaadin-popover-overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class PopoverOverlay extends PopoverOverlayMixin(DirMixin(ThemableMixin(PolylitM
render() {
return html`
<div id="backdrop" part="backdrop" hidden ?hidden="${!this.withBackdrop}"></div>
<div part="overlay" id="overlay">
<div part="overlay" id="overlay" tabindex="0">
<div part="content" id="content"><slot></slot></div>
</div>
`;
Expand Down
18 changes: 18 additions & 0 deletions packages/popover/src/vaadin-popover.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,24 @@ declare class Popover extends PopoverPositionMixin(
*/
renderer: PopoverRenderer | null | undefined;

/**
* Set to true to disable closing popover overlay on outside click.
* Closing on outside click only works when the popover is modal.
*
* @attr {boolean} no-close-on-outside-click
*/
noCloseOnOutsideClick: boolean;

/**
* Set to true to disable closing popover overlay on Escape press.
* When the popover is modal, pressing Escape anywhere in the
* document closes the overlay. Otherwise, only Escape press
* from the popover itself or its target closes the overlay.
*
* @attr {boolean} no-close-on-esc
*/
noCloseOnEsc: boolean;

/**
* Requests an update for the content of the popover.
* While performing the update, it invokes the renderer passed in the `renderer` property.
Expand Down
56 changes: 56 additions & 0 deletions packages/popover/src/vaadin-popover.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,30 @@ class Popover extends PopoverPositionMixin(
type: Object,
},

/**
* Set to true to disable closing popover overlay on outside click.
* Closing on outside click only works when the popover is modal.
*
* @attr {boolean} no-close-on-outside-click
*/
noCloseOnOutsideClick: {
type: Boolean,
value: false,
},

/**
* Set to true to disable closing popover overlay on Escape press.
* When the popover is modal, pressing Escape anywhere in the
* document closes the overlay. Otherwise, only Escape press
* from the popover itself or its target closes the overlay.
*
* @attr {boolean} no-close-on-esc
*/
noCloseOnEsc: {
type: Boolean,
value: false,
},

/** @private */
_opened: {
type: Boolean,
Expand All @@ -60,6 +84,7 @@ class Popover extends PopoverPositionMixin(
constructor() {
super();
this.__onTargetClick = this.__onTargetClick.bind(this);
this.__onTargetKeydown = this.__onTargetKeydown.bind(this);
}

/** @protected */
Expand All @@ -79,6 +104,8 @@ class Popover extends PopoverPositionMixin(
.horizontalAlign="${this.__computeHorizontalAlign(effectivePosition)}"
.verticalAlign="${this.__computeVerticalAlign(effectivePosition)}"
@opened-changed="${this.__onOpenedChanged}"
@vaadin-overlay-escape-press="${this.__onEscapePress}"
@vaadin-overlay-outside-click="${this.__onOutsideClick}"
></vaadin-popover-overlay>
`;
}
Expand Down Expand Up @@ -118,6 +145,7 @@ class Popover extends PopoverPositionMixin(
*/
_addTargetListeners(target) {
target.addEventListener('click', this.__onTargetClick);
target.addEventListener('keydown', this.__onTargetKeydown);
}

/**
Expand All @@ -127,17 +155,45 @@ class Popover extends PopoverPositionMixin(
*/
_removeTargetListeners(target) {
target.removeEventListener('click', this.__onTargetClick);
target.removeEventListener('keydown', this.__onTargetKeydown);
}

/** @private */
__onTargetClick() {
this._opened = !this._opened;
}

/** @private */
__onTargetKeydown(event) {
if (event.key === 'Escape' && !this.noCloseOnEsc) {
this._opened = false;
}
}

/** @private */
__onOpenedChanged(event) {
this._opened = event.detail.value;
}

/**
* Close the popover if `noCloseOnEsc` isn't set to true.
* @private
*/
__onEscapePress(e) {
if (this.noCloseOnEsc) {
e.preventDefault();
}
}

/**
* Close the popover if `noCloseOnOutsideClick` isn't set to true.
* @private
*/
__onOutsideClick(e) {
if (this.noCloseOnOutsideClick) {
e.preventDefault();
}
}
}

defineCustomElement(Popover);
Expand Down
63 changes: 59 additions & 4 deletions packages/popover/test/basic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,22 +149,77 @@ describe('popover', () => {
expect(overlay.opened).to.be.false;
});

it('should close overlay on Escape press by default', async () => {
it('should not close on outside click if noCloseOnOutsideClick is true', async () => {
popover.noCloseOnOutsideClick = true;
await nextUpdate(popover);

target.click();
await nextRender();

esc(document.body);
outsideClick();
await nextRender();
expect(overlay.opened).to.be.false;
expect(overlay.opened).to.be.true;
});

it('should close overlay on when popover is detached', async () => {
it('should close overlay when popover is detached', async () => {
target.click();
await nextRender();

popover.remove();
await nextRender();
expect(overlay.opened).to.be.false;
});

describe('Escape press', () => {
beforeEach(async () => {
target.click();
await nextRender();
});

it('should close overlay on global Escape press by default', async () => {
esc(document.body);
await nextRender();
expect(overlay.opened).to.be.false;
});

it('should close overlay on internal Escape press by default', async () => {
esc(overlay.$.overlay);
await nextRender();
expect(overlay.opened).to.be.false;
});

it('should close overlay on target Escape press by default', async () => {
esc(target);
await nextRender();
expect(overlay.opened).to.be.false;
});

it('should not close on global Escape press if noCloseOnEsc is true', async () => {
popover.noCloseOnEsc = true;
await nextUpdate(popover);

esc(document.body);
await nextRender();
expect(overlay.opened).to.be.true;
});

it('should not close overlay on internal Escape if noCloseOnEsc is true', async () => {
popover.noCloseOnEsc = true;
await nextUpdate(popover);

esc(overlay.$.overlay);
await nextRender();
expect(overlay.opened).to.be.true;
});

it('should not close overlay on target Escape if noCloseOnEsc is true', async () => {
popover.noCloseOnEsc = true;
await nextUpdate(popover);

esc(target);
await nextRender();
expect(overlay.opened).to.be.true;
});
});
});
});
2 changes: 2 additions & 0 deletions packages/popover/test/typings/popover.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,5 @@ assertType<HTMLElement | undefined>(popover.target);
assertType<PopoverPosition>(popover.position);
assertType<PopoverRenderer | null | undefined>(popover.renderer);
assertType<string>(popover.overlayClass);
assertType<boolean>(popover.noCloseOnEsc);
assertType<boolean>(popover.noCloseOnOutsideClick);

0 comments on commit 645f463

Please sign in to comment.