-
Notifications
You must be signed in to change notification settings - Fork 20
feat(components): create post popover trigger component #6209
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
base: main
Are you sure you want to change the base?
Changes from all commits
cdfafa6
62ed4c5
8c216b1
372b494
81c4ae6
ac1c470
ea844ff
dfef7fc
92f601d
a5dcbeb
9c8927d
ba7e526
21337c3
5627456
aaf4e2f
daf42fd
35d716d
b3c3cf0
dda1b48
c1eb08f
35b400a
705a1c3
710c1e5
c37aef4
b9c6188
937194d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,7 @@ | ||||||
--- | ||||||
'@swisspost/design-system-components-angular-workspace': patch | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This package is private so there is no need to add it to the changeset |
||||||
'@swisspost/design-system-documentation': patch | ||||||
'@swisspost/design-system-components': patch | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This is a breaking change |
||||||
--- | ||||||
|
||||||
Introduced `<post-popover-trigger>` web component to replace the previous `data-popover-target` implementation. |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is already a test file for the popover in the |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
describe('Popover', () => { | ||
beforeEach(() => { | ||
cy.visit('/popover'); | ||
cy.get('post-popover[data-hydrated]').as('popover'); | ||
cy.get('post-popovercontainer[data-hydrated]').as('popovercontainer'); | ||
cy.get('#popoverContent').as('popoverContent'); | ||
cy.get('post-popover-trigger[data-hydrated][for="popover-one"]') | ||
.children() | ||
.first() | ||
.as('trigger'); | ||
}); | ||
|
||
it('should contain an HTML element inside the trigger, not just plain text', () => { | ||
cy.get('post-popover-trigger[data-hydrated][for="popover-one"]') | ||
.children() | ||
.should('have.length.at.least', 1); | ||
}); | ||
it('should show up on click', () => { | ||
cy.get('@popoverContent').should('not.be.visible'); | ||
cy.get('@trigger').should('have.attr', 'aria-expanded', 'false'); | ||
cy.get('@trigger').click(); | ||
cy.get('@popover').should('be.visible'); | ||
cy.get('@trigger').should('have.attr', 'aria-expanded', 'true'); | ||
// Void click light dismiss does not work in cypress for closing | ||
}); | ||
|
||
it('should show up when clicking on a nested element inside the trigger', () => { | ||
// Modify trigger by adding a nested span | ||
cy.get('@trigger').then($trigger => { | ||
const originalText = $trigger.text(); | ||
$trigger.html(`<span class="nested-element">${originalText}</span>`); | ||
}); | ||
|
||
cy.get('@popoverContent').should('not.be.visible'); | ||
cy.get('@trigger').should('have.attr', 'aria-expanded', 'false'); | ||
cy.get('.nested-element').click(); | ||
cy.get('@popoverContent').should('be.visible'); | ||
cy.get('@trigger').should('have.attr', 'aria-expanded', 'true'); | ||
cy.get('.btn-close').click(); | ||
cy.get('@popoverContent').should('not.be.visible'); | ||
cy.get('@trigger').should('have.attr', 'aria-expanded', 'false'); | ||
}); | ||
|
||
it('should show up when clicking on a deeply nested element inside the trigger', () => { | ||
// Set up a trigger with a deeply nested structure | ||
cy.get('@trigger').then($trigger => { | ||
const originalText = $trigger.text(); | ||
$trigger.html(` | ||
<div class="level-1"> | ||
<div class="level-2"> | ||
<span class="level-3">${originalText}</span> | ||
</div> | ||
</div> | ||
`); | ||
}); | ||
|
||
cy.get('@popoverContent').should('not.be.visible'); | ||
cy.get('.level-3').click(); | ||
cy.get('@popoverContent').should('be.visible'); | ||
cy.get('@trigger').should('have.attr', 'aria-expanded', 'true'); | ||
}); | ||
|
||
it('should close on X click', () => { | ||
cy.get('@trigger').click(); | ||
cy.get('@popoverContent').should('be.visible'); | ||
cy.get('.btn-close').click(); | ||
cy.get('@popoverContent').should('not.be.visible'); | ||
}); | ||
|
||
it('should open on enter and close on escape', () => { | ||
cy.get('@popoverContent').should('not.be.visible'); | ||
cy.get('@trigger').focus().type('{enter}'); | ||
cy.get('@popoverContent').should('be.visible'); | ||
}); | ||
|
||
it('should open and close with the API', () => { | ||
Promise.all([cy.get('@trigger'), cy.get('@popoverContent')]) | ||
.then(([$trigger, $popover]: [JQuery<HTMLButtonElement>, JQuery<HTMLPostPopoverElement>]) => [ | ||
$trigger.get(0), | ||
$popover.get(0), | ||
]) | ||
.then(([trigger, popover]: [HTMLButtonElement, HTMLPostPopoverElement]) => { | ||
cy.get('@popoverContent').should('not.be.visible'); | ||
popover.show(trigger); | ||
cy.get('@popoverContent').should('be.visible'); | ||
popover.hide(); | ||
cy.get('@popoverContent').should('not.be.visible'); | ||
popover.toggle(trigger); | ||
cy.get('@popoverContent').should('be.visible'); | ||
popover.toggle(trigger); | ||
cy.get('@popoverContent').should('not.be.visible'); | ||
}); | ||
|
||
it('should switch position', () => { | ||
cy.get('post-popover').invoke('attr', 'placement', 'top'); | ||
cy.get('@popoverContent').should('not.be.visible'); | ||
|
||
Promise.all([cy.get('@trigger'), cy.get('@popover')]) | ||
.then( | ||
([$trigger, $popover]: [JQuery<HTMLButtonElement>, JQuery<HTMLPostPopoverElement>]) => [ | ||
$trigger.get(0), | ||
$popover.get(0), | ||
], | ||
) | ||
.then(([trigger, popover]: [HTMLButtonElement, HTMLPostPopoverElement]) => { | ||
const t = trigger.getBoundingClientRect(); | ||
const p = popover.getBoundingClientRect(); | ||
expect(t.top < p.top); | ||
}); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
<h2>Popover</h2> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All web components are documented within the |
||
|
||
<post-popover-trigger for="popover-one"> | ||
<button class="btn btn-secondary">Popover Trigger</button> | ||
</post-popover-trigger> | ||
<post-popover | ||
class="palette palette-accent" | ||
id="popover-one" | ||
placement="top" | ||
close-button-caption="Close" | ||
arrow="" | ||
><div id="popoverContent"> | ||
<h2 class="h6">Optional title</h2> | ||
<span class="mb-0"> | ||
A longer message that needs more time to read. <a href="#">Links</a> are also possible.</span | ||
> | ||
</div> | ||
</post-popover> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import { Component } from '@angular/core'; | ||
import { CommonModule } from '@angular/common'; | ||
import { ReactiveFormsModule } from '@angular/forms'; | ||
import { PostComponentsModule } from 'components'; | ||
|
||
@Component({ | ||
selector: 'popover-page', | ||
templateUrl: './popover.component.html', | ||
imports: [CommonModule, ReactiveFormsModule, PostComponentsModule], | ||
}) | ||
export class PopoverComponent {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,10 +7,19 @@ describe('popover', { baseUrl: null, includeShadowDom: true }, () => { | |
cy.get('post-popover[data-hydrated]'); | ||
|
||
// 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('post-popover-trigger[data-hydrated][for="popover-one"]') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be good to also test:
|
||
.children() | ||
.first() | ||
.as('trigger'); | ||
cy.get('#testtext').as('popover'); | ||
}); | ||
|
||
it('should contain an HTML element inside the trigger, not just plain text', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you could more specifically check that there is focusable content within the trigger |
||
cy.get('post-popover-trigger[data-hydrated][for="popover-one"]') | ||
.children() | ||
.should('have.length.at.least', 1); | ||
}); | ||
|
||
it('should show up on click', () => { | ||
cy.get('@popover').should('not.be.visible'); | ||
cy.get('@trigger').should('have.attr', 'aria-expanded', 'false'); | ||
|
@@ -63,12 +72,10 @@ describe('popover', { baseUrl: null, includeShadowDom: true }, () => { | |
cy.get('@popover').should('not.be.visible'); | ||
}); | ||
|
||
it('should open and close on enter', () => { | ||
it('should open on enter', () => { | ||
cy.get('@popover').should('not.be.visible'); | ||
cy.get('@trigger').focus().type('{enter}'); | ||
cy.get('@popover').should('be.visible'); | ||
cy.get('@trigger').type('{enter}'); | ||
cy.get('@popover').should('not.be.visible'); | ||
}); | ||
|
||
it('should open and close with the API', () => { | ||
|
@@ -119,7 +126,8 @@ describe('popover', { baseUrl: null, includeShadowDom: true }, () => { | |
cy.get('post-popover[data-hydrated]'); | ||
|
||
// 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('post-popover-trigger[data-hydrated][for="popover-one"]').as('trigger'); | ||
|
||
cy.injectAxe(); | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
:host { | ||
cursor: pointer; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not related to this file but I see two remaining usage of
data-popover-target
, one in the angular consumer app, and one in the nextjs-integration package.