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)
Change History (13)
Changed 10 years ago by
| Attachment: | dialog.txt added |
|---|
comment:1 Changed 10 years ago by
| Component: | General → Dijit |
|---|---|
| Owner: | anonymous deleted |
comment:2 Changed 10 years ago by
| Summary: | Calling Dialog.destroy() does not remove the overlay → Dialog: Calling Dialog.destroy() does not remove the overlay |
|---|
comment:3 Changed 10 years ago by
| Milestone: | tbd → 1.3 |
|---|---|
| Owner: | set to bill |
| Status: | new → assigned |
| Summary: | Dialog: Calling Dialog.destroy() does not remove the overlay → Dialog: 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
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
[cla][patch] added a dijit._underlay.hide(); in the uninitialize method.
I changed a bit the test page
Changed 10 years ago by
| Attachment: | Dialog.js.diff added |
|---|
comment:7 follow-up: 8 Changed 10 years ago by
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 Changed 10 years ago by
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
| Attachment: | Dialog.js.diff2 added |
|---|
comment:9 Changed 10 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
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.