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

feat: fire closed event when Dialog overlay is closed #7471

Merged
merged 6 commits into from
Jun 4, 2024

Conversation

DiegoCardoso
Copy link
Contributor

Description

Add a new closed event that is triggered after the overlay is closed

Part of #7422

Type of change

  • Bugfix
  • Feature

packages/dialog/src/vaadin-dialog.d.ts Outdated Show resolved Hide resolved
describe('closed event', () => {
it('should dispatch closed event when closed', async () => {
const closedSpy = sinon.spy();
listenOnce(dialog, 'closed', closedSpy);
Copy link
Member

Choose a reason for hiding this comment

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

Could use addEventListener instead of listenOnce. When using listenOnce, expect(closedSpy.calledOnce).to.be.true; wouldn't fail even if the event was dispatched twice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member

@tomivirkki tomivirkki Jun 4, 2024

Choose a reason for hiding this comment

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

There's no need to remove the event listener at the end of the test. Each test run gets a fresh new dialog instance.

Copy link
Member

@web-padawan web-padawan left a comment

Choose a reason for hiding this comment

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

Consider also adding the event to typings tests.

@DiegoCardoso
Copy link
Contributor Author

Consider also adding the event to typings tests.

Typing test added.

@@ -102,6 +109,7 @@ export type DialogEventMap = DialogCustomEventMap & HTMLElementEventMap;
*
* @fires {CustomEvent} resize - Fired when the dialog resize is finished.
* @fires {CustomEvent} opened-changed - Fired when the `opened` property changes.
* @fires {CustomEvent} closed - Fired when the overlay is closed.
Copy link
Member

Choose a reason for hiding this comment

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

This line is still different compared to .js file: it says "overlay is closed" vs "dialog is closed".

Copy link

sonarcloud bot commented Jun 4, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.5.0.beta2 and is also targeting the upcoming stable 24.5.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants