Skip to content

fix: Improve dialog close handling - #7614

Merged
GZolla merged 3 commits into
mainfrom
gzolla/dialog-handle-close
Oct 15, 2026
Merged

GZolla merged 3 commits into
mainfrom
gzolla/dialog-handle-close

Conversation

@GZolla

@GZolla GZolla commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

GAUD-10785

If a dialog is closed and immediately disconnected(i.e. a parent component is removed while listening to an event dispatched from the same operation as the closure). Then the dialog is never re-rendered, the native dialog does not close and the handler does not run. The disconnected callback check is also ignored since the dialog is closed as it was disconnected.

This means that in that scenario the handler never runs and the body cannot be scrolled. So this PR changes the disconnected callback check so it ensures the handler is always run after a dialog is opened, be that it's closed or disconnected.

@GZolla
GZolla requested a review from a team as a code owner October 13, 2026 21:47
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the PR! 🎉

We've deployed an automatic preview for this PR - you can see your changes here:

URL https://live.d2l.dev/prs/BrightspaceUI/core/pr-7614/

Note

The build needs to finish before your changes are deployed.
Changes to the PR will automatically update the instance.


// If the dialog is disconnected before the close animation finishes
if (this.opened) {
if (!this.#closeHandled) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a way to unit test this?

expect(elem.opened).to.be.false;
expect(document.body.style.overflow).to.equal('');
});
it(`should re-enable body scrolling when closed and disconnected${useNative ? ' for native dialogs' : ''}`, async() => {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This test fails on main when using native dialogs

@dlockhart dlockhart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice!

@GZolla
GZolla merged commit e55295e into main Oct 15, 2026
11 of 12 checks passed
@GZolla
GZolla deleted the gzolla/dialog-handle-close branch October 15, 2026 14:45
@d2l-github-release-tokens

Copy link
Copy Markdown

🎉 This PR is included in version 3.331.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants