Dialog widget fix

0 views
Skip to first unread message

sw45859

unread,
Nov 20, 2009, 10:24:07 PM11/20/09
to jQuery UI Development
the diff is below, basically the issue was that the "open" event was
not getting the event or ui arguments sent to it.


@@ -216,7 +216,7 @@
return self;
},

- open: function() {
+ open: function(event) {
if (this._isOpen) { return; }

var self = this,
@@ -258,7 +258,7 @@
.filter(':first')
.focus();

- self._trigger('open');
+ self._trigger('open', event, self);
self._isOpen = true;

return self;

Scott González

unread,
Nov 20, 2009, 10:29:57 PM11/20/09
to jquery...@googlegroups.com
This is not a bug, the dialog open event is functioning as intended and as documented. Requesting the ability to pass an event to the open method can be discussed, but I'd like to hear a good argument for why this should be supported. As for the ui parameter, there is no information to pass, so nothing should be passed - plugins instances are never exposed via the ui hash.



--

You received this message because you are subscribed to the Google Groups "jQuery UI Development" group.
To post to this group, send email to jquery...@googlegroups.com.
To unsubscribe from this group, send email to jquery-ui-de...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/jquery-ui-dev?hl=.



sw45859

unread,
Nov 20, 2009, 10:31:22 PM11/20/09
to jQuery UI Development
then i guess it's time to update the documentation.


Code examples

Supply a callback function to handle the open event as an init option.

$('.selector').dialog({
open: function(event, ui) { ... }
});

Bind to the open event by type: dialogopen.

$('.selector').bind('dialogopen', function(event, ui) {
...
});

sw45859

unread,
Nov 20, 2009, 10:35:20 PM11/20/09
to jQuery UI Development
sorry for the double post, but the reason for me needing this
personally was to access the ui.element. which is supposed to be
accessible according to my understanding of the developer guidelines,
maybe i'm just wrong. anyhow this particular thing gave me what i
needed which was to access the ui-dialog-content ( which is not
accessible otherwise ), although i couldn't use ui-element, it ended
up being ui.uiDialog

Scott González

unread,
Nov 20, 2009, 10:44:01 PM11/20/09
to jquery...@googlegroups.com
The event object is definitely passed - you can't possibly trigger an
event without an event. The event just doesn't have an original event
that points to a native event.

Also, there should never be any mention of plugin instance properties
in current docs as these are all internal.

As for getting the dialog element, it is the event target, so it's
readily available.

sw45859

unread,
Nov 20, 2009, 10:58:18 PM11/20/09
to jQuery UI Development
ok my bad, i was able to access the content element using e.target. so
i guess this has been reduced to a documentation update to prevent
others from assuming the same thing i did, because i did add ui as the
second argument and ui.element comes up as undefined.

Scott González

unread,
Nov 21, 2009, 8:17:56 AM11/21/09
to jquery...@googlegroups.com
Where are you getting the impression that ui.element would exist?

sw45859

unread,
Nov 21, 2009, 5:20:52 PM11/21/09
to jQuery UI Development
without looking into the code.

from the ui developer guidelines:

Available properties for each instance:

* options: The options for this widget instance, usually a mix of
defaults with settings provided by the user
* element: A jQuery object always containing a single DOMElement,
which can be accessed with this.element[0].

from the dialog documentation:

Code examples

Supply a callback function to handle the open event as an init option.

$('.selector').dialog({
open: function(event, ui) { ... }
});

so to me it would seem that if the ui object is passed around that
there would be a "(this|ui).element" attached to it somewhere, even if
thats not how it's supposed to be ( and probably isn't ), it does make
sense to allow accessing the ui version of the element via that var,
at least for the dialog. where it is used in my script is when i'm
making a dynamic dialog, without using an element that's already in
the dom. like the code below. yes it could be static but this is the
start of a generic modal confirmation dialog which will be called via
a function to populate the title/content, and some content will be
ajax retrieved.

my argument for why this would be good to have "it offers a simple way
to support ajax filled dialogs without having to build an ajax
function in the widget itself to do it". granted you can use e.target,
but that's not documented anywhere in the dialog so it's not
"painfully obvious" to people like myself who always glance at docs
rather than read them.

$('#deleteProduct').click(function (){
var $selfButton = $(this);
$('<div></div>').dialog({
autoOpen: true,
width: 300,
modal: true,
resizable: false,
allowClose: false,
title: 'Delete Product Confirm',
open: function (e){
$(e.target).html('Are you sure you want to delete this product?');
},
close: function (){
$(this).dialog('destroy');
},
buttons: {
'Delete Product': function(){
window.location = js_app_link
('app=products&appPage=default&action=deleteProductConfirm&products_id='
+ $selfButton.attr('products_id'));
},
'Don\'t Delete': function(){
$(this).dialog('destroy');
}
}
});
return false;
});

Scott González

unread,
Nov 21, 2009, 6:13:31 PM11/21/09
to jquery...@googlegroups.com
On Sat, Nov 21, 2009 at 5:20 PM, sw45859 <swalke...@gmail.com> wrote:
without looking into the code.

from the ui developer guidelines:

Available properties for each instance:

   * options: The options for this widget instance, usually a mix of
defaults with settings provided by the user
   * element: A jQuery object always containing a single DOMElement,
which can be accessed with this.element[0].

Yes, this is the *developer* guidelines, not the *user* guidelines. This is about developing plugins and how the plugin instances work. Most users don't interact with the plugin instances, they always go through the single exposed jQuery function. For those who want to interact with the plugin instance directly, they can but anything that's not publicly documented is fair game to change without warning.

from the dialog documentation:

Code examples

Supply a callback function to handle the open event as an init option.

   $('.selector').dialog({
      open: function(event, ui) { ... }
   });

This ui hash has nothing to do with the plugin instance. This is simply a hash of data relevant to the event, which in some cases is nothing.
 
so to me it would seem that if the ui object is passed around that
there would be a "(this|ui).element" attached to it somewhere, even if
thats not how it's supposed to be ( and probably isn't ), it does make
sense to allow accessing the ui version of the element via that var,
at least for the dialog. where it is used in my script is when i'm
making a dynamic dialog, without using an element that's already in
the dom. like the code below. yes it could be static but this is the
start of a generic modal confirmation dialog which will be called via
a function to populate the title/content, and some content will be
ajax retrieved.

When there is data provided in the ui hash, it is listed in the documentation. We can't account for all assumptions that users will make about what will and won't be available somewhere. There is no documentation, code, tutorials, etc. indicating that your assumption would be correct, so I don't really know what to say about that.
 
my argument for why this would be good to have "it offers a simple way
to support ajax filled dialogs without having to build an ajax
function in the widget itself to do it". granted you can use e.target,
but that's not documented anywhere in the dialog so it's not
"painfully obvious" to people like myself who always glance at docs
rather than read them.

Ajax support is already dead simple. The documentation clearly states that you can bind event handlers, so what would you expect to be in event.target if not the element corresponding to the plugin? Also, the context of the callback is the dialog element (which you're apparently already aware of based on your close callback).

Reply all
Reply to author
Forward
0 new messages