Opened 10 years ago

Closed 10 years ago

#8672 closed defect (fixed)

Dialog: Calling Dialog.destroy() does not hide the overlay

Reported by: coldfire22x Owned by: bill
Priority: high Milestone: 1.3
Component: Dijit Version: 1.3.0b1
Keywords: Dialog destroy Cc:
Blocked By: Blocking:

Description

See summary and attached. 1.2.3 removes the overlay on the call to destroy().

Furthermore, I've tried calling hide() and then destroy(), but that does not work either.

Attachments (4)

dialog.txt (1.6 KB) - added by coldfire22x 10 years ago.
Dialog.js.diff (662 bytes) - added by nic 10 years ago.
dialog_bug.html (1.3 KB) - added by nic 10 years ago.
test page
Dialog.js.diff2 (911 bytes) - added by nic 10 years ago.

Download all attachments as: .zip

Change History (13)

Changed 10 years ago by coldfire22x

Attachment: dialog.txt added

comment:1 Changed 10 years ago by Adam Peller

Component: GeneralDijit
Owner: anonymous deleted

comment:2 Changed 10 years ago by bill

Summary: Calling Dialog.destroy() does not remove the overlayDialog: Calling Dialog.destroy() does not remove the overlay

Dialog was refactored so everyone shares a single underlay, as documented in the release notes, so destroying a Dialog isn't supposed to remove the underlay.

What exactly is the issue here? Are you saying that the overlay still appears on the screen? Because I tried test_Dialog.html and it's working correctly for me.

comment:3 Changed 10 years ago by bill

Milestone: tbd1.3
Owner: set to bill
Status: newassigned
Summary: Dialog: Calling Dialog.destroy() does not remove the overlayDialog: Calling Dialog.destroy() does not hide the overlay

Just talked to coldfire on IRC. The actual issue is that the underlay isn't getting hidden, in the case when the dialog is destroyed right after the dialog is hidden:

dlg.hide();
dlg.destroy();

Maybe there's another issue too if you destroy the dialog without hiding it; have to check that too.

Anyway I think the first issue is because the underlay is hidden at the end of the hide animation, which isn't getting a chance to complete.

comment:4 Changed 10 years ago by Scott

bill: that's precisely what the problem is: using the test case in #8737 (duplicate of this bug), calling dlg.hide(); and dlg.destroy; doesn't hide the underlay. For what it's worth, this was not a problem in 1.1.1 and I only noticed it when upgrading to 1.3.0b1.

comment:5 Changed 10 years ago by nic

[cla][patch] added a dijit._underlay.hide(); in the uninitialize method.
I changed a bit the test page

Changed 10 years ago by nic

Attachment: Dialog.js.diff added

Changed 10 years ago by nic

Attachment: dialog_bug.html added

test page

comment:6 Changed 10 years ago by bill

See also #8737 which has test case attached.

comment:7 Changed 10 years ago by bill

Oh ignore my comment above, hadn't read through all the recent comments in this ticket.

Nic, thanks for the patch... one small issue: what if Dialog2 is showing and then we delete Dialog1? Won't that incorrectly hide the underlay for Dialog2?

comment:8 in reply to:  7 Changed 10 years ago by nic

Replying to bill:

Oh ignore my comment above, hadn't read through all the recent comments in this ticket.

Nic, thanks for the patch... one small issue: what if Dialog2 is showing and then we delete Dialog1? Won't that incorrectly hide the underlay for Dialog2?

Ugh, you are right. I never used two Dialogs at once! :(
I'll take a look tomorrow.

Changed 10 years ago by nic

Attachment: Dialog.js.diff2 added

comment:9 Changed 10 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [16838]) Hide underlay if dialog is destroyed while being shown. Fixes #8672 !strict. Patch from Nic Rizzo (CLA on file).

Note: See TracTickets for help on using tickets.