Skip to content

Commit

Permalink
feat: make popover modeless by default, add modality properties (#7412)
Browse files Browse the repository at this point in the history
  • Loading branch information
web-padawan authored May 16, 2024
1 parent fb8c32c commit dc9b8c7
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 3 deletions.
4 changes: 4 additions & 0 deletions packages/popover/src/vaadin-popover-overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ class PopoverOverlay extends PopoverOverlayMixin(DirMixin(ThemableMixin(PolylitM
return [
overlayStyles,
css`
:host([modeless][with-backdrop]) [part='backdrop'] {
pointer-events: none;
}
:host([position^='top'][top-aligned]) [part='overlay'],
:host([position^='bottom'][top-aligned]) [part='overlay'] {
margin-top: var(--vaadin-popover-offset-top, 0);
Expand Down
16 changes: 15 additions & 1 deletion packages/popover/src/vaadin-popover.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,15 @@ declare class Popover extends PopoverPositionMixin(
*/
renderer: PopoverRenderer | null | undefined;

/**
* When true, the popover prevents interacting with background elements
* by setting `pointer-events` style on the document body to `none`.
* This also enables trapping focus inside the overlay.
*/
modal: boolean;

/**
* 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
*/
Expand All @@ -50,6 +56,14 @@ declare class Popover extends PopoverPositionMixin(
*/
noCloseOnEsc: boolean;

/**
* When true, the overlay has a backdrop (modality curtain) on top of the
* underlying page content, covering the whole viewport.
*
* @attr {boolean} with-backdrop
*/
withBackdrop: 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
51 changes: 50 additions & 1 deletion packages/popover/src/vaadin-popover.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,18 @@ class Popover extends PopoverPositionMixin(
type: Object,
},

/**
* When true, the popover prevents interacting with background elements
* by setting `pointer-events` style on the document body to `none`.
* This also enables trapping focus inside the overlay.
*/
modal: {
type: Boolean,
value: false,
},

/**
* 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
*/
Expand All @@ -73,6 +82,17 @@ class Popover extends PopoverPositionMixin(
value: false,
},

/**
* When true, the overlay has a backdrop (modality curtain) on top of the
* underlying page content, covering the whole viewport.
*
* @attr {boolean} with-backdrop
*/
withBackdrop: {
type: Boolean,
value: false,
},

/** @private */
_opened: {
type: Boolean,
Expand All @@ -83,6 +103,7 @@ class Popover extends PopoverPositionMixin(

constructor() {
super();
this.__onGlobalClick = this.__onGlobalClick.bind(this);
this.__onTargetClick = this.__onTargetClick.bind(this);
this.__onTargetKeydown = this.__onTargetKeydown.bind(this);
}
Expand All @@ -99,6 +120,9 @@ class Popover extends PopoverPositionMixin(
.positionTarget="${this.target}"
.position="${effectivePosition}"
.opened="${this._opened}"
.modeless="${!this.modal}"
.focusTrap="${this.modal}"
.withBackdrop="${this.withBackdrop}"
?no-horizontal-overlap="${this.__computeNoHorizontalOverlap(effectivePosition)}"
?no-vertical-overlap="${this.__computeNoVerticalOverlap(effectivePosition)}"
.horizontalAlign="${this.__computeHorizontalAlign(effectivePosition)}"
Expand Down Expand Up @@ -131,10 +155,19 @@ class Popover extends PopoverPositionMixin(
this._overlayElement = this.shadowRoot.querySelector('vaadin-popover-overlay');
}

/** @protected */
connectedCallback() {
super.connectedCallback();

document.addEventListener('click', this.__onGlobalClick, true);
}

/** @protected */
disconnectedCallback() {
super.disconnectedCallback();

document.removeEventListener('click', this.__onGlobalClick, true);

this._opened = false;
}

Expand All @@ -158,6 +191,22 @@ class Popover extends PopoverPositionMixin(
target.removeEventListener('keydown', this.__onTargetKeydown);
}

/**
* Overlay's global outside click listener doesn't work when
* the overlay is modeless, so we use a separate listener.
* @private
*/
__onGlobalClick(event) {
if (
this._opened &&
!this.modal &&
!event.composedPath().some((el) => el === this._overlayElement || el === this.target) &&
!this.noCloseOnOutsideClick
) {
this._opened = false;
}
}

/** @private */
__onTargetClick() {
this._opened = !this._opened;
Expand Down
84 changes: 83 additions & 1 deletion packages/popover/test/basic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,38 @@ describe('popover', () => {
});
});

describe('overlay properties', () => {
it('should set modeless on the overlay by default', () => {
expect(overlay.modeless).to.be.true;
});

it('should set modeless on the overlay to false when modal is true', async () => {
popover.modal = true;
await nextUpdate(popover);
expect(overlay.modeless).to.be.false;
});

it('should not set focusTrap on the overlay by default', () => {
expect(overlay.modeless).to.be.true;
});

it('should set focusTrap on the overlay to true when modal is true', async () => {
popover.modal = true;
await nextUpdate(popover);
expect(overlay.focusTrap).to.be.true;
});

it('should propagate withBackdrop property to the overlay', async () => {
popover.withBackdrop = true;
await nextUpdate(popover);
expect(overlay.withBackdrop).to.be.true;

popover.withBackdrop = false;
await nextUpdate(popover);
expect(overlay.withBackdrop).to.be.false;
});
});

describe('interactions', () => {
let target;

Expand Down Expand Up @@ -149,6 +181,18 @@ describe('popover', () => {
expect(overlay.opened).to.be.false;
});

it('should close overlay on outside click when modal is true', async () => {
popover.modal = true;
await nextUpdate(popover);

target.click();
await nextRender();

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

it('should not close on outside click if noCloseOnOutsideClick is true', async () => {
popover.noCloseOnOutsideClick = true;
await nextUpdate(popover);
Expand All @@ -170,13 +214,30 @@ describe('popover', () => {
expect(overlay.opened).to.be.false;
});

it('should remove document click listener when popover is detached', async () => {
const spy = sinon.spy(document, 'removeEventListener');
popover.remove();
await nextRender();
expect(spy).to.be.called;
expect(spy.firstCall.args[0]).to.equal('click');
});

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

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

it('should close overlay on global Escape press when modal is true', async () => {
popover.modal = true;
await nextUpdate(popover);

esc(document.body);
await nextRender();
expect(overlay.opened).to.be.false;
Expand All @@ -195,6 +256,7 @@ describe('popover', () => {
});

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

Expand All @@ -221,5 +283,25 @@ describe('popover', () => {
expect(overlay.opened).to.be.true;
});
});

describe('backdrop', () => {
beforeEach(async () => {
popover.withBackdrop = true;
await nextUpdate(popover);

target.click();
await nextRender();
});

it('should set pointer-events on backdrop to none when non modal', () => {
expect(getComputedStyle(overlay.$.backdrop).pointerEvents).to.equal('none');
});

it('should set pointer-events on backdrop to auto when modal', async () => {
popover.modal = true;
await nextUpdate(popover);
expect(getComputedStyle(overlay.$.backdrop).pointerEvents).to.equal('auto');
});
});
});
});
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,5 +23,7 @@ assertType<HTMLElement | undefined>(popover.target);
assertType<PopoverPosition>(popover.position);
assertType<PopoverRenderer | null | undefined>(popover.renderer);
assertType<string>(popover.overlayClass);
assertType<boolean>(popover.modal);
assertType<boolean>(popover.withBackdrop);
assertType<boolean>(popover.noCloseOnEsc);
assertType<boolean>(popover.noCloseOnOutsideClick);

0 comments on commit dc9b8c7

Please sign in to comment.