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

fix(popover): stay open on click when using polyfill #3211

Merged
merged 4 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
5 changes: 5 additions & 0 deletions .changeset/beige-cooks-switch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@swisspost/design-system-components': patch
---

Fixed an issue with popovers on Firefox ESR that unexpectedly closed popovers when clicking on content.
2 changes: 1 addition & 1 deletion packages/components/cypress/e2e/popover.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ describe('popover', { baseUrl: null, includeShadowDom: true }, () => {
cy.visit('./cypress/fixtures/post-popover.test.html');
// Aria-expanded is set by the web component, therefore it's a good measure to indicate the component is ready
cy.get('[data-popover-target="popover-one"][aria-expanded]').as('trigger');
cy.get('div.popover-container').as('popover');
cy.get('#testtext').as('popover');
});

it('should show up on click', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/cypress/e2e/popovercontainer.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ describe('popovercontainer', { baseUrl: null, includeShadowDom: true }, () => {
// There is no dedicated docs page for the popovercontainer
cy.visit('./cypress/fixtures/post-popover.test.html');
cy.get('[data-popover-target="popover-one"][aria-expanded]').as('trigger');
cy.get('div.popover-container').as('container');
cy.get('#testtext').as('container');
});

it('should show up on click', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/components/cypress/e2e/tooltip.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ describe('tooltips', { baseUrl: null, includeShadowDom: true }, () => {
cy.visit('./cypress/fixtures/post-tooltip.test.html');
cy.get('#target1').as('target1');
cy.get('#target2').as('target2');
cy.get('#tooltip-one').find('div[popover]').as('tooltip');
cy.get('#tooltip-one').find('post-popovercontainer[popover]').as('tooltip');
});

it('should display a tooltip', () => {
Expand Down Expand Up @@ -54,7 +54,7 @@ describe('tooltips', { baseUrl: null, includeShadowDom: true }, () => {
cy.visit('./cypress/fixtures/post-tooltip.test.html');
cy.get('#target-child-element').as('target');
cy.get('#target-child-element span').as('target-child');
cy.get('#tooltip-one').find('div[popover]').as('tooltip');
cy.get('#tooltip-one').find('post-popovercontainer[popover]').as('tooltip');
});

it('should show tooltip on hovered child element', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<body>
<button data-popover-target="popover-one">toggle</button>
<post-popover id="popover-one">
<p>This is a test</p>
<p id="testtext">This is a test</p>
</post-popover>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,7 @@
padding: 0.5em;
flex-grow: 1;
}

.btn-close {
color: inherit;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
}
}

.popover {
:where(post-popovercontainer) {
@include elevation-mx.elevation('elevation-3');

position: fixed;
Expand All @@ -36,36 +36,36 @@

// Keeps the little arrow visible
overflow: visible;
}

.arrow {
$arrow-size: 0.5825rem;
position: absolute;
width: $arrow-size;
height: $arrow-size;
background-color: inherit;
rotate: 45deg;
pointer-events: none;
z-index: -1;
.arrow {
$arrow-size: 0.5825rem;
position: absolute;
width: $arrow-size;
height: $arrow-size;
background-color: inherit;
rotate: 45deg;
pointer-events: none;
z-index: -1;

// Create transparent border to be styled by and for the high contrast mode
&.top {
border-left: 2px solid transparent;
border-top: 2px solid transparent;
}
// Create transparent border to be styled by and for the high contrast mode
&.top {
border-left: 2px solid transparent;
border-top: 2px solid transparent;
}

&.right {
border-right: 2px solid transparent;
border-top: 2px solid transparent;
}
&.right {
border-right: 2px solid transparent;
border-top: 2px solid transparent;
}

&.left {
border-left: 2px solid transparent;
border-bottom: 2px solid transparent;
}
&.left {
border-left: 2px solid transparent;
border-bottom: 2px solid transparent;
}

&.bottom {
border-right: 2px solid transparent;
border-bottom: 2px solid transparent;
&.bottom {
border-right: 2px solid transparent;
border-bottom: 2px solid transparent;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, Element, Event, EventEmitter, h, Host, Method, Prop } from '@stencil/core';
import { Component, Element, Event, EventEmitter, Host, Method, Prop, h } from '@stencil/core';
import {
arrow,
autoUpdate,
Expand Down Expand Up @@ -39,11 +39,9 @@ export type PostPopoverElement = HTMLElement & PopoverElement;
@Component({
tag: 'post-popovercontainer',
styleUrl: 'post-popovercontainer.scss',
shadow: true,
})
export class PostPopovercontainer {
@Element() host: HTMLPostPopovercontainerElement;
private popoverRef: PostPopoverElement;
private arrowRef: HTMLElement;
private eventTarget: Element;
private clearAutoUpdate: () => void;
Expand All @@ -67,16 +65,11 @@ export class PostPopovercontainer {
@Prop() readonly arrow?: boolean = false;

componentDidLoad() {
this.popoverRef.setAttribute('popover', '');
this.popoverRef.addEventListener('beforetoggle', this.handleToggle.bind(this));
this.host.setAttribute('popover', '');
this.host.addEventListener('beforetoggle', this.handleToggle.bind(this));
}

disconnectedCallback() {
if (this.popoverRef)
this.popoverRef.removeEventListener('beforetoggle', e =>
this.toggle(e.target as HTMLPostPopovercontainerElement),
);

if (typeof this.clearAutoUpdate === 'function') this.clearAutoUpdate();
}

Expand All @@ -89,7 +82,7 @@ export class PostPopovercontainer {
if (!this.toggleTimeoutId) {
this.eventTarget = target;
this.calculatePosition();
this.popoverRef.showPopover();
this.host.showPopover();
}
}

Expand All @@ -100,7 +93,7 @@ export class PostPopovercontainer {
async hide() {
if (!this.toggleTimeoutId) {
this.eventTarget = null;
this.popoverRef.hidePopover();
this.host.hidePopover();
}
}

Expand All @@ -115,10 +108,10 @@ export class PostPopovercontainer {
if (!this.toggleTimeoutId) {
this.eventTarget = target;
this.calculatePosition();
this.popoverRef.togglePopover(force);
this.host.togglePopover(force);
this.toggleTimeoutId = null;
}
return this.popoverRef.matches(':popover-open');
return this.host.matches(':where(:popover-open, .popover-open');
}

/**
Expand All @@ -145,7 +138,7 @@ export class PostPopovercontainer {
private startAutoupdates() {
this.clearAutoUpdate = autoUpdate(
this.eventTarget,
this.popoverRef,
this.host,
this.calculatePosition.bind(this),
);
}
Expand Down Expand Up @@ -182,15 +175,15 @@ export class PostPopovercontainer {
y,
middlewareData,
placement: currentPlacement,
} = await computePosition(this.eventTarget, this.popoverRef, {
} = await computePosition(this.eventTarget, this.host, {
placement: this.placement || 'top',
strategy: 'fixed',
middleware,
});

// Tooltip
this.popoverRef.style.left = `${x}px`;
this.popoverRef.style.top = `${y}px`;
this.host.style.left = `${x}px`;
this.host.style.top = `${y}px`;

// Arrow
if (this.arrow) {
Expand All @@ -215,21 +208,15 @@ export class PostPopovercontainer {
render() {
return (
<Host data-version={version}>
<div
class="popover"
part="popover"
ref={(el: HTMLDivElement & PostPopoverElement) => (this.popoverRef = el)}
>
{this.arrow && (
<span
class="arrow"
ref={el => {
this.arrowRef = el;
}}
></span>
)}
<slot></slot>
</div>
{this.arrow && (
<span
class="arrow"
ref={el => {
this.arrowRef = el;
}}
></span>
)}
<slot></slot>
</Host>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,6 @@ Type: `Promise<boolean>`
| | Default slot for placing content inside the popovercontainer. |


## Shadow Parts

| Part | Description |
| ----------- | ----------- |
| `"popover"` | |


## Dependencies

### Used by
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,12 @@
}

post-popovercontainer {
&::part(popover) {
padding: spacing.$size-micro spacing.$size-mini;
max-width: 2 * spacing.$size-bigger-giant - spacing.$size-mini;
min-height: spacing.$size-regular;
}
padding: spacing.$size-micro spacing.$size-mini;
max-width: 2 * spacing.$size-bigger-giant - spacing.$size-mini;
min-height: spacing.$size-regular;

// Creates a safe space around the popovercontainer for save pointer crossing between trigger and tooltip
&.has-arrow::part(popover) {
&[arrow] {
&::after {
position: absolute;
content: '';
Expand Down
5 changes: 5 additions & 0 deletions packages/components/src/index.html
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes can be reverted.

Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,10 @@
</head>
<body>
<div class="p-5">Add your component here and start developing!</div>
<button data-popover-target="bla">poopver</button>
<post-popover id="bla">bla</post-popover>

<button data-tooltip-target="tool">tooltip</button>
<post-tooltip id="tool">tool</post-tooltip>
</body>
</html>