Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add properties to control popover closing behavior #7414

Merged
merged 2 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
}

tomivirkki marked this conversation as resolved.
Show resolved Hide resolved
/** @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);
Loading