Skip to content

Commit b999b2d

Browse files
myrta2302hugomslvalizedebray
authored
feat(components): create post popover trigger component (#6209)
## 📄 Description This PR introduces a `<post-popover-trigger>` web component to replace the former `data-popover-target=" "` implementation. Aria attribute and focus management logic is now handeled within the trigger component. Documentation and tests are updated accordingly. ## 📝 Checklist - ✅ My code follows the style guidelines of this project - 🛠️ I have performed a self-review of my own code - 📄 I have made corresponding changes to the documentation - ⚠️ My changes generate no new warnings or errors - 🧪 I have udpated the existing tests - ✔️ New and existing unit tests pass locally with my changes - Manual Accessibility Test --------- Co-authored-by: Machado da Silva Hugo <Hugo.macdasil@gmail.com> Co-authored-by: Machado da Silva Hugo <hugomslv@users.noreply.github.com> Co-authored-by: Hugo <170833805+hugomslv@users.noreply.github.com> Co-authored-by: Alizé Debray <33580481+alizedebray@users.noreply.github.com>
1 parent d9a1da8 commit b999b2d

File tree

20 files changed

+373
-160
lines changed

20 files changed

+373
-160
lines changed

.changeset/forty-jokes-sin.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@swisspost/design-system-documentation': patch
3+
'@swisspost/design-system-components': major
4+
---
5+
6+
Introduced `<post-popover-trigger>` web component to replace the previous `data-popover-target` implementation.

packages/components-angular/projects/consumer-app/src/app/routes/home/home.component.html

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,20 @@ <h2>Post Logo</h2>
5555

5656
<div class="my-24">
5757
<h2>Post Popover</h2>
58-
<button class="btn btn-secondary btn-large" data-popover-target="popover-one">
59-
Click here to see a popover
60-
</button>
61-
<post-popover closeButtonCaption="Close" id="popover-one" placement="top" arrow="">
62-
<h2 class="h6">Optional title</h2>
63-
64-
<p class="mb-0">
65-
A longer message that needs more time to read.
66-
<a href="#">Links</a>
67-
are also possible.
68-
</p>
58+
<post-popover-trigger for="popover-one">
59+
<button type="button" class="btn btn-secondary">Popover Trigger</button>
60+
</post-popover-trigger>
61+
<post-popover
62+
class="palette palette-accent"
63+
id="popover-one"
64+
placement="top"
65+
close-button-caption="Close"
66+
><div id="popoverContent">
67+
<h2 class="h6">Optional title</h2>
68+
<p class="mb-0">
69+
A longer message that needs more time to read. <a href="#">Links</a> are also possible.
70+
</p>
71+
</div>
6972
</post-popover>
7073
</div>
7174

packages/components-angular/projects/consumer-app/src/app/routes/home/home.component.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
PostIcon,
1111
PostLogo,
1212
PostPopover,
13+
PostPopoverTrigger,
1314
PostPopovercontainer,
1415
PostRating,
1516
PostTabs,
@@ -33,15 +34,15 @@ import {
3334
PostIcon,
3435
PostLogo,
3536
PostPopover,
37+
PostPopoverTrigger,
3638
PostPopovercontainer,
3739
PostRating,
3840
PostTabs,
3941
PostTabHeader,
4042
PostTabPanel,
4143
PostTooltipTrigger,
42-
]
44+
],
4345
})
44-
4546
export class HomeComponent {
4647
isCollapsed = false;
4748
}

packages/components/cypress/e2e/popover.cy.ts

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,27 @@ describe('popover', { baseUrl: null, includeShadowDom: true }, () => {
33
beforeEach(() => {
44
cy.visit('./cypress/fixtures/post-popover.test.html');
55

6-
// Ensure the component is hydrated, which is necessary to ensure the component is ready for interaction
76
cy.get('post-popover[data-hydrated]');
87

9-
// Aria-expanded is set by the web component, therefore it's a good measure to indicate the component is ready
10-
cy.get('[data-popover-target="popover-one"][aria-expanded]').as('trigger');
8+
cy.get('post-popover-trigger[data-hydrated][for="popover-one"]')
9+
.children()
10+
.first()
11+
.as('trigger');
1112
cy.get('#testtext').as('popover');
13+
14+
// Alternative popover trigger with no interactive element
15+
cy.get('post-popover-trigger[data-hydrated][for="popover-two"]')
16+
.children()
17+
.first()
18+
.as('trigger2');
19+
});
20+
21+
it('if the element inside the trigger is not interactive, it should at least have a set tabindex="0" and role="button"', () => {
22+
cy.get('@trigger2').then($child => {
23+
const child = $child[0];
24+
cy.wrap(child).should('have.attr', 'tabindex', '0');
25+
cy.wrap(child).should('have.attr', 'role', 'button');
26+
});
1227
});
1328

1429
it('should show up on click', () => {
@@ -17,6 +32,7 @@ describe('popover', { baseUrl: null, includeShadowDom: true }, () => {
1732
cy.get('@trigger').click();
1833
cy.get('@popover').should('be.visible');
1934
cy.get('@trigger').should('have.attr', 'aria-expanded', 'true');
35+
2036
// Void click light dismiss does not work in cypress for closing
2137
});
2238

@@ -42,12 +58,12 @@ describe('popover', { baseUrl: null, includeShadowDom: true }, () => {
4258
cy.get('@trigger').then($trigger => {
4359
const originalText = $trigger.text();
4460
$trigger.html(`
45-
<div class="level-1">
46-
<div class="level-2">
47-
<span class="level-3">${originalText}</span>
48-
</div>
49-
</div>
50-
`);
61+
<div class="level-1">
62+
<div class="level-2">
63+
<span class="level-3">${originalText}</span>
64+
</div>
65+
</div>
66+
`);
5167
});
5268

5369
cy.get('@popover').should('not.be.visible');
@@ -63,12 +79,16 @@ describe('popover', { baseUrl: null, includeShadowDom: true }, () => {
6379
cy.get('@popover').should('not.be.visible');
6480
});
6581

66-
it('should open and close on enter', () => {
82+
it('should open on enter and first focusable element should be focused', () => {
6783
cy.get('@popover').should('not.be.visible');
6884
cy.get('@trigger').focus().type('{enter}');
6985
cy.get('@popover').should('be.visible');
70-
cy.get('@trigger').type('{enter}');
71-
cy.get('@popover').should('not.be.visible');
86+
87+
// find the first focusable element (e.g., button, input, link, etc.)
88+
cy.get('@popover')
89+
.find('button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])')
90+
.first()
91+
.should('have.focus');
7292
});
7393

7494
it('should open and close with the API', () => {
@@ -114,13 +134,6 @@ describe('popover', { baseUrl: null, includeShadowDom: true }, () => {
114134
describe('Accessibility', () => {
115135
beforeEach(() => {
116136
cy.visit('./cypress/fixtures/post-popover.test.html');
117-
118-
// Ensure the component is hydrated, which is necessary to ensure the component is ready for interaction
119-
cy.get('post-popover[data-hydrated]');
120-
121-
// Aria-expanded is set by the web component, therefore it's a good measure to indicate the component is ready
122-
cy.get('[data-popover-target="popover-one"][aria-expanded]').as('trigger');
123-
124137
cy.injectAxe();
125138
});
126139

packages/components/cypress/e2e/popovercontainer.cy.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,17 @@ describe('popovercontainer', { baseUrl: null, includeShadowDom: true }, () => {
33
const selector = isPopoverSupported() ? ':popover-open' : '.\\:popover-open';
44

55
beforeEach(() => {
6-
// There is no dedicated docs page for the popovercontainer
76
cy.visit('./cypress/fixtures/post-popover.test.html');
8-
cy.get('[data-popover-target="popover-one"][aria-expanded]').as('trigger');
7+
8+
// Ensure the component is hydrated, which is necessary to ensure the component is ready for interaction
9+
cy.get('post-popover[data-hydrated]');
10+
11+
// Aria-expanded is set by the web component, therefore it's a good measure to indicate the component is ready
12+
cy.get('post-popover-trigger[data-hydrated][for="popover-one"]')
13+
.children()
14+
.first()
15+
.as('trigger');
16+
917
cy.get('#testtext').as('container');
1018
});
1119

packages/components/cypress/fixtures/post-popover.test.html

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,16 @@
1111
<script src="../../dist/post-components/post-components.esm.js" type="module"></script>
1212
</head>
1313
<body>
14-
<button data-popover-target="popover-one">toggle</button>
14+
<post-popover-trigger for="popover-one">
15+
<button type="button" class="btn btn-secondary">Popover Trigger</button>
16+
</post-popover-trigger>
1517
<post-popover id="popover-one" close-button-caption="Close Popover">
16-
<p id="testtext">This is a test</p>
18+
<p id="testtext">This is a <a href="">test</a></p>
1719
</post-popover>
20+
21+
<!-- Alternative trigger case with no interactive element inside the trigger -->
22+
<post-popover-trigger for="popover-two">
23+
<div>Popover Trigger</div>
24+
</post-popover-trigger>
1825
</body>
1926
</html>

packages/components/src/components.d.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -386,16 +386,22 @@ export namespace Components {
386386
"placement"?: Placement;
387387
/**
388388
* Programmatically display the popover
389-
* @param target An element with [data-popover-target="id"] where the popover should be shown
389+
* @param target A focusable element inside the <post-popover-trigger> component that controls the popover
390390
*/
391391
"show": (target: HTMLElement) => Promise<void>;
392392
/**
393393
* Toggle popover display
394-
* @param target An element with [data-popover-target="id"] where the popover should be anchored to
394+
* @param target A focusable element inside the <post-popover-trigger> component that controls the popover
395395
* @param force Pass true to always show or false to always hide
396396
*/
397397
"toggle": (target: HTMLElement, force?: boolean) => Promise<void>;
398398
}
399+
interface PostPopoverTrigger {
400+
/**
401+
* ID of the popover element that this trigger is linked to. Used to open and close the popover.
402+
*/
403+
"for": string;
404+
}
399405
interface PostPopovercontainer {
400406
/**
401407
* Animation style
@@ -440,12 +446,12 @@ export namespace Components {
440446
"safeSpace"?: 'triangle' | 'trapezoid';
441447
/**
442448
* Programmatically display the popovercontainer
443-
* @param target An element with [data-popover-target="id"] where the popovercontainer should be shown
449+
* @param target A focusable element inside the <post-popover-trigger> component that controls the popover
444450
*/
445451
"show": (target: HTMLElement) => Promise<void>;
446452
/**
447453
* Toggle popovercontainer display
448-
* @param target An element with [data-popover-target="id"] where the popovercontainer should be shown
454+
* @param target A focusable element inside the <post-popover-trigger> component that controls the popover
449455
* @param force Pass true to always show or false to always hide
450456
*/
451457
"toggle": (target: HTMLElement, force?: boolean) => Promise<boolean>;
@@ -826,6 +832,12 @@ declare global {
826832
prototype: HTMLPostPopoverElement;
827833
new (): HTMLPostPopoverElement;
828834
};
835+
interface HTMLPostPopoverTriggerElement extends Components.PostPopoverTrigger, HTMLStencilElement {
836+
}
837+
var HTMLPostPopoverTriggerElement: {
838+
prototype: HTMLPostPopoverTriggerElement;
839+
new (): HTMLPostPopoverTriggerElement;
840+
};
829841
interface HTMLPostPopovercontainerElementEventMap {
830842
"postBeforeShow": { first?: boolean };
831843
"postShow": { first?: boolean };
@@ -940,6 +952,7 @@ declare global {
940952
"post-menu-item": HTMLPostMenuItemElement;
941953
"post-menu-trigger": HTMLPostMenuTriggerElement;
942954
"post-popover": HTMLPostPopoverElement;
955+
"post-popover-trigger": HTMLPostPopoverTriggerElement;
943956
"post-popovercontainer": HTMLPostPopovercontainerElement;
944957
"post-rating": HTMLPostRatingElement;
945958
"post-tab-header": HTMLPostTabHeaderElement;
@@ -1275,6 +1288,12 @@ declare namespace LocalJSX {
12751288
*/
12761289
"placement"?: Placement;
12771290
}
1291+
interface PostPopoverTrigger {
1292+
/**
1293+
* ID of the popover element that this trigger is linked to. Used to open and close the popover.
1294+
*/
1295+
"for": string;
1296+
}
12781297
interface PostPopovercontainer {
12791298
/**
12801299
* Animation style
@@ -1449,6 +1468,7 @@ declare namespace LocalJSX {
14491468
"post-menu-item": PostMenuItem;
14501469
"post-menu-trigger": PostMenuTrigger;
14511470
"post-popover": PostPopover;
1471+
"post-popover-trigger": PostPopoverTrigger;
14521472
"post-popovercontainer": PostPopovercontainer;
14531473
"post-rating": PostRating;
14541474
"post-tab-header": PostTabHeader;
@@ -1496,6 +1516,7 @@ declare module "@stencil/core" {
14961516
"post-menu-item": LocalJSX.PostMenuItem & JSXBase.HTMLAttributes<HTMLPostMenuItemElement>;
14971517
"post-menu-trigger": LocalJSX.PostMenuTrigger & JSXBase.HTMLAttributes<HTMLPostMenuTriggerElement>;
14981518
"post-popover": LocalJSX.PostPopover & JSXBase.HTMLAttributes<HTMLPostPopoverElement>;
1519+
"post-popover-trigger": LocalJSX.PostPopoverTrigger & JSXBase.HTMLAttributes<HTMLPostPopoverTriggerElement>;
14991520
"post-popovercontainer": LocalJSX.PostPopovercontainer & JSXBase.HTMLAttributes<HTMLPostPopovercontainerElement>;
15001521
"post-rating": LocalJSX.PostRating & JSXBase.HTMLAttributes<HTMLPostRatingElement>;
15011522
"post-tab-header": LocalJSX.PostTabHeader & JSXBase.HTMLAttributes<HTMLPostTabHeaderElement>;
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:host {
2+
cursor: pointer;
3+
}

0 commit comments

Comments
 (0)