Skip to content

Commit

Permalink
fix(modal): prevent odsClose event on unmount if closed
Browse files Browse the repository at this point in the history
  • Loading branch information
@dpellier
dpellier committed Oct 23, 2024
1 parent 65a0505 commit 17e4c4c
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 107 deletions.
8 changes: 7 additions & 1 deletion packages/ods/src/components/modal/src/components/ods-modal/ods-modal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,14 @@ export class OdsModal {
disconnectedCallback(): void {
document.body.style.removeProperty('overflow');
this.modalDialog?.close?.();

// TODO we should not emit on disconnectedCallback as the emitter does not exists anymore
// this should be refactored with some use-case example on FAQ, especially regarding react-testing-library
// (do this only when we have working react-testing-library on our side to validate every use-case)
if (this.isOpen) {
this.odsClose.emit();
}
this.isOpen = false;
this.odsClose.emit();
}

handleBackdropClick(event: MouseEvent): void {
Expand Down
158 changes: 84 additions & 74 deletions packages/ods/src/components/modal/src/index.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,80 +8,6 @@
<script type='module' src='/build/ods-modal.esm.js'></script>
<script nomodule src='/build/ods-modal.js'></script>
<link rel="stylesheet" href="/build/ods-modal.css">
</head>

<body>
<ods-button id="button-1" label="Modal 1" icon="face-wink"></ods-button>
<ods-button id="button-2" label="Modal 2" icon="shield" variant="outline"></ods-button>
<ods-button id="button-3" label="Modal 3" icon="globe" color="critical"></ods-button>
<ods-button id="button-4" label="Modal 4" icon="star" color="critical" variant="outline"></ods-button>

<ods-modal id="modal-1" color="information" is-dismissible="true" is-open>
<ods-text preset="heading-4" class="heading">
Hello, world!
</ods-text>
<ods-text preset="span">
Lorem ipsum dolor sit amet, consectetur adipiscing elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
</ods-text>
<ods-button class="button" slot="actions" label="Learn more" icon="book" variant="outline"></ods-button>
<ods-button class="button" slot="actions" label="Activate" icon="shield"></ods-button>
</ods-modal>

<ods-modal id="modal-2" color="warning" is-dismissible="true">
<ods-text preset="heading-4" class="heading">
Hello, world!
</ods-text>
<ods-text preset="span">
Lorem ipsum dolor sit amet, consectetur adipiscing elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
</ods-text>
</ods-modal>

<ods-modal id="modal-3" color="critical" is-dismissible="false">
<ods-text preset="heading-4" class="heading">
Hello, world!
</ods-text>
<ods-text preset="span">
Lorem ipsum dolor sit amet, consectetur adipiscing elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
</ods-text>
<ods-button class="button" slot="actions" label="Learn more" icon="book" variant="outline"></ods-button>
<ods-button class="button" slot="actions" label="Activate" icon="shield"></ods-button>
</ods-modal>

<ods-modal id="modal-4" color="success" is-dismissible="true">
<ods-text preset="heading-4" class="heading">
Hello, world!
</ods-text>
<ods-text preset="span">
Lorem ipsum dolor sit amet, consectetur adipiscing elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
</ods-text>
</ods-modal>

<script>
const button1 = document.getElementById('button-1');
const button2 = document.getElementById('button-2');
const button3 = document.getElementById('button-3');
const button4 = document.getElementById('button-4');
const modal1 = document.getElementById('modal-1');
const modal2 = document.getElementById('modal-2');
const modal3 = document.getElementById('modal-3');
const modal4 = document.getElementById('modal-4');

button1.addEventListener('click', () => {
modal1.open();
});

button2.addEventListener('click', () => {
modal2.open();
});

button3.addEventListener('click', () => {
modal3.open();
});

button4.addEventListener('click', () => {
modal4.open();
});
</script>

<style>
* {
Expand All @@ -97,5 +23,89 @@
padding-top: 24px;
}
</style>
</head>

<body>
<!-- <ods-button id="button-1" label="Modal 1" icon="face-wink"></ods-button>-->
<!-- <ods-button id="button-2" label="Modal 2" icon="shield" variant="outline"></ods-button>-->
<!-- <ods-button id="button-3" label="Modal 3" icon="globe" color="critical"></ods-button>-->
<!-- <ods-button id="button-4" label="Modal 4" icon="star" color="critical" variant="outline"></ods-button>-->

<!-- <ods-modal id="modal-1" color="information" is-dismissible="true" is-open>-->
<!-- <ods-text preset="heading-4" class="heading">-->
<!-- Hello, world!-->
<!-- </ods-text>-->
<!-- <ods-text preset="span">-->
<!-- Lorem ipsum dolor sit amet, consectetur adipiscing elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.-->
<!-- </ods-text>-->
<!-- <ods-button class="button" slot="actions" label="Learn more" icon="book" variant="outline"></ods-button>-->
<!-- <ods-button class="button" slot="actions" label="Activate" icon="shield"></ods-button>-->
<!-- </ods-modal>-->

<!-- <ods-modal id="modal-2" color="warning" is-dismissible="true">-->
<!-- <ods-text preset="heading-4" class="heading">-->
<!-- Hello, world!-->
<!-- </ods-text>-->
<!-- <ods-text preset="span">-->
<!-- Lorem ipsum dolor sit amet, consectetur adipiscing elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.-->
<!-- </ods-text>-->
<!-- </ods-modal>-->

<!-- <ods-modal id="modal-3" color="critical" is-dismissible="false">-->
<!-- <ods-text preset="heading-4" class="heading">-->
<!-- Hello, world!-->
<!-- </ods-text>-->
<!-- <ods-text preset="span">-->
<!-- Lorem ipsum dolor sit amet, consectetur adipiscing elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.-->
<!-- </ods-text>-->
<!-- <ods-button class="button" slot="actions" label="Learn more" icon="book" variant="outline"></ods-button>-->
<!-- <ods-button class="button" slot="actions" label="Activate" icon="shield"></ods-button>-->
<!-- </ods-modal>-->

<!-- <ods-modal id="modal-4" color="success" is-dismissible="true">-->
<!-- <ods-text preset="heading-4" class="heading">-->
<!-- Hello, world!-->
<!-- </ods-text>-->
<!-- <ods-text preset="span">-->
<!-- Lorem ipsum dolor sit amet, consectetur adipiscing elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.-->
<!-- </ods-text>-->
<!-- </ods-modal>-->

<!-- <script>-->
<!-- const button1 = document.getElementById('button-1');-->
<!-- const button2 = document.getElementById('button-2');-->
<!-- const button3 = document.getElementById('button-3');-->
<!-- const button4 = document.getElementById('button-4');-->
<!-- const modal1 = document.getElementById('modal-1');-->
<!-- const modal2 = document.getElementById('modal-2');-->
<!-- const modal3 = document.getElementById('modal-3');-->
<!-- const modal4 = document.getElementById('modal-4');-->

<!-- button1.addEventListener('click', () => {-->
<!-- modal1.open();-->
<!-- });-->

<!-- button2.addEventListener('click', () => {-->
<!-- modal2.open();-->
<!-- });-->

<!-- button3.addEventListener('click', () => {-->
<!-- modal3.open();-->
<!-- });-->

<!-- button4.addEventListener('click', () => {-->
<!-- modal4.open();-->
<!-- });-->
<!-- </script>-->

<ods-modal id="my-modal" is-open>Modal test</ods-modal>

<script>
const modalEl = document.querySelector('#my-modal');

modalEl.addEventListener('odsClose', () => {
console.log('Close event received');
});
</script>
</body>
</html>
98 changes: 67 additions & 31 deletions packages/ods/src/components/modal/tests/behaviour/ods-modal.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,51 +27,87 @@ describe('ods-modal behaviour', () => {
});
}

it('should trigger open event when opening', async() => {
await setup('<ods-modal><ods-text>Hello, world!</ods-text></ods-modal>');
const openSpy = await page.spyOnEvent('odsOpen');
const modal = await page.find('ods-modal');
describe('events', () => {
describe('odsClose', () => {
it('should trigger when closing', async() => {
await setup('<ods-modal is-open><ods-text>Hello, world!</ods-text></ods-modal>');
const closeSpy = await page.spyOnEvent('odsClose');
const modal = await page.find('ods-modal');

await modal.callMethod('open');
await modal.callMethod('close');
await waitForAnimationEnd();

expect(openSpy).toHaveReceivedEventTimes(1);
});
expect(closeSpy).toHaveReceivedEventTimes(1);
});

it('should trigger close event when closing', async() => {
await setup('<ods-modal is-open><ods-text>Hello, world!</ods-text></ods-modal>');
const closeSpy = await page.spyOnEvent('odsClose');
const modal = await page.find('ods-modal');
it('should trigger when unmounting if opened', async() => {
await setup('<ods-modal is-open><ods-text>Hello, world!</ods-text></ods-modal>');
const modalHandle = await page.waitForSelector('ods-modal');
const modal = await page.find('ods-modal');
const closeSpy = await modal.spyOnEvent('odsClose');

await modal.callMethod('close');
await waitForAnimationEnd();
await modalHandle?.evaluate((element) => element.remove());
await page.waitForChanges();

expect(closeSpy).toHaveReceivedEventTimes(1);
});
expect(closeSpy).toHaveReceivedEventTimes(1);
});

it('should open on element open method call', async() => {
await setup('<ods-modal><ods-text>Hello, world!</ods-text></ods-modal>');
const modal = await page.find('ods-modal');
it('should not trigger when unmounting if closed', async() => {
await setup('<ods-modal><ods-text>Hello, world!</ods-text></ods-modal>');
const modalHandle = await page.waitForSelector('ods-modal');
const modal = await page.find('ods-modal');
const closeSpy = await modal.spyOnEvent('odsClose');

await modal.callMethod('open');
await page.waitForChanges();
await modalHandle?.evaluate((element) => element.remove());
await page.waitForChanges();

const isOpen = await page.evaluate(() => {
const dialog = document.querySelector('ods-modal')?.shadowRoot?.querySelector('.ods-modal__dialog') as HTMLDialogElement;
return dialog && dialog.hasAttribute('open');
expect(closeSpy).not.toHaveReceivedEvent();
});
});

expect(isOpen).toBe(true);
describe('odsOpen', () => {
it('should trigger when opening', async() => {
await setup('<ods-modal><ods-text>Hello, world!</ods-text></ods-modal>');
const openSpy = await page.spyOnEvent('odsOpen');
const modal = await page.find('ods-modal');

await modal.callMethod('open');

expect(openSpy).toHaveReceivedEventTimes(1);
});
});
});

it('should add a close animation on element close method call', async() => {
await setup('<ods-modal is-open><ods-text>Hello, world!</ods-text></ods-modal>');
const modal = await page.find('ods-modal');
describe('methods', () => {
describe('close', () => {
it('should add a close animation on element close method call', async() => {
await setup('<ods-modal is-open><ods-text>Hello, world!</ods-text></ods-modal>');
const modal = await page.find('ods-modal');

await modal.callMethod('close');
await page.waitForChanges();
await modal.callMethod('close');
await page.waitForChanges();

const closeAnimation = await page.find('ods-modal >>> .ods-modal__dialog--close-animation');

expect(closeAnimation).not.toBeNull();
});
});

const closeAnimation = await page.find('ods-modal >>> .ods-modal__dialog--close-animation');
describe('open', () => {
it('should open on element open method call', async() => {
await setup('<ods-modal><ods-text>Hello, world!</ods-text></ods-modal>');
const modal = await page.find('ods-modal');

expect(closeAnimation).not.toBeNull();
await modal.callMethod('open');
await page.waitForChanges();

const isOpen = await page.evaluate(() => {
const dialog = document.querySelector('ods-modal')?.shadowRoot?.querySelector('.ods-modal__dialog') as HTMLDialogElement;
return dialog && dialog.hasAttribute('open');
});

expect(isOpen).toBe(true);
});
});
});
});
2 changes: 1 addition & 1 deletion packages/storybook/stories/components/modal/documentation.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ They can be dismissable or not.
donts={[
'- Trigger a Modal as a result of a first Modal actions',
'- Use a Modal if the informational content is very long within a Modal: consider alternatives if that is your case',
<span>- Use a **Modal** only to notify users about a successful completion of their actions: use <StorybookLink kind="ODS Components/Message" label="Message" story="Documentation" /> component</span>,
<span>- Use a Modal only to notify users about a successful completion of their actions: use <StorybookLink kind="ODS Components/Message" label="Message" story="Documentation" /> component</span>,
]}
dos={[
'- Use a Modal to display information the user needs to respond to',
Expand Down

0 comments on commit 17e4c4c

Please sign in to comment.