Opened 10 years ago

Closed 6 years ago

#9670 closed enhancement (duplicate)

Dialog: closing should be cancellable

Reported by: Aleksey Rechinskiy Owned by:
Priority: low Milestone: future
Component: Dijit Version: 1.3.2
Keywords: DIalog hide cancel Cc:
Blocked By: Blocking:

Description

And one more proposition to dijit.Dialog:

Consider the following Dialog use-case:

Lets imagine the page, that makes an important long-runnning AJAX requests and must prevent a user to iteract with the page content during that requests. Each request should also be cancellable by user after confirm()ing "Are you sure you want to cancel this request? (not recommended)"

An obvious Dojo component to use to create a cancellable modal message "request is running, please wait" is a dijit.Dialog with the message and a button "Cancel request". When user clicks on the button, confirm() is fired and if the user agrees, the request is cancelled. If the user disagrees, Dialog should be running as before. Looks good?

There is a small issue with the Dialog I don't know how to overcome: if user clicks on X button in Dialog's title bar, Dialog will close without a chance to prevent closing (when user say "No, I don't want to cancel the request, I've just decided to wait until it ends, let it run.")

X button is tied to onCancel() function. onCancel() has a hide() function connected. There is no room for Dialog closing prevention in this model.

It is possible to override hide() function on the Dialog's instance to make it fire confirm() and call original hide() if YES, and dismiss the call if NO. But it would be much better, if the Dialog::hide() function will call a some kind of callback shouldIClose() and if it returns true, do real hide(), but if false - dismiss the call.

The proposal looks very easy to implement, but it would widen a range of possible use-cases of dijit.Dialog.

Thanks.

Change History (12)

comment:1 Changed 10 years ago by Aleksey Rechinskiy

Forgot to say: it is easy to inherit Dialog and override hide() to support cancelling, but it looks to me like using a sledge-hammer to crack nuts.

For example, Dialog can have a fnShouldClose property nulled by default. The property should be checked right in start of hide() and if it is a function - call it and see if it returns non-false value. If someone wants to add cancelling functionality to the Dialog instance, he should just set this property to a necessary callback.

comment:2 Changed 10 years ago by bill

Hmm, well we did do something similar for TitlePane in [18285], I guess it's doable.

I don't want to modify had the hide() method works, as developers should still be able to close a dialog programatically, but the idea would be to hide the [x] icon for closing the dialog, and disable ESC key processing to close the dialog.

comment:3 Changed 10 years ago by Aleksey Rechinskiy

Mmm, yes, I agree, the Dialog should be closeable by a developer...

To hide the [x] icon is a good solution.

Thanks.

comment:4 Changed 10 years ago by dante

Isn't there a modal="true" param? Or is that only in dojox.widget.Dialog. Hiding the [x] with css is a single line rule

.dijitDialogCloseIcon { display:none } 

should do it, no?

comment:5 Changed 10 years ago by Aleksey Rechinskiy

Hi Dante!

modal='true' param is for dojox.widget.Dialog indeed. I think, it would be nice to transfer it to the its parent dijit.Dialog.

It is possible to hide [x] button with CSS rule, yes... It doesn't allow to change the [x] state at run-time, but it works for most use-cases indeed (including the use-case I described earlier).

Well... Looks like there is no strict need for this ticket, but... Well.. Okay, I think there are a lot of more important things to do in Dojo :)

Thanks for reply.

comment:6 Changed 10 years ago by bill

Milestone: tbd1.5

comment:7 Changed 10 years ago by bill

Summary: dijit.Dialog closing should be cancellableDialog: closing should be cancellable

Note that the ESC key processing is in popup.js IIRC, so that makes things a little difficult.

comment:8 Changed 9 years ago by Adam Peller

Milestone: 1.51.6

comment:9 Changed 9 years ago by bill

Milestone: 1.6future

comment:10 Changed 7 years ago by gstefani

Even though a property would be much easier and cleaner, I think we can already avoid the esc key with the sort-of-new dojo/aspect now

The following code will "disable" the esc key while it keeps other keyboard shortcuts working.

aspect.around(dialog, "_onKey", lang.hitch(this, function(originalMethod){
 return function(evt) {
  if (evt.charOrCode !== keys.ESCAPE) {
	originalMethod.apply(this, arguments);
  }
 };		     	
}));

comment:11 Changed 7 years ago by bill

Priority: highlow

Similar to #8345, except this is asking for a callback method to accept/reject closing the dialog. Not sure whether or not #8345 by itself is sufficient.

comment:12 Changed 6 years ago by bill

Resolution: duplicate
Status: newclosed

Duplicate of #8345.
I think that ticket is sufficient.

Note: See TracTickets for help on using tickets.