Skip to content

[1.11] Widget: Implement instance method on the bridge to return widget instance #902

Closed
wants to merge 3 commits into from

3 participants

@gnarf
jQuery Foundation member
gnarf commented Jan 30, 2013

A new method on the widget bridge that gives you access to the widget instance without relying on the location it's stored in data not changing.

Not implemented exactly like the ticket described, but there was some fun logic in the bridge that needed to be overridden to make it return instance.

http://jqbug.com/ui/9030

@scottgonzalez
jQuery Foundation member

@gnarf37 Can you rebase this? We're ready to land it in master.

@gnarf
jQuery Foundation member
gnarf commented Mar 16, 2013

rebased

@jzaefferer jzaefferer commented on the diff Mar 18, 2013
demos/autocomplete/combobox.html
@@ -152,7 +152,7 @@
this._delay(function() {
this.input.tooltip( "close" ).attr( "title", "" );
}, 2500 );
- this.input.data( "ui-autocomplete" ).term = "";
+ this.input.autocomplete( "instance" ).term = "";
@jzaefferer
jQuery Foundation member
jzaefferer added a note Mar 18, 2013

This seems to need more changes, the demo is broken. Shows the select element and throws Uncaught ReferenceError: input is not defined at combobox.html:95

@scottgonzalez
jQuery Foundation member

Rebase again? Both of those errors were fixed in the past few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jzaefferer jzaefferer commented on the diff Mar 18, 2013
tests/unit/widget/widget_core.js
@@ -625,6 +625,22 @@ test( ".widget() - overriden", function() {
deepEqual( wrapper[0], $( "<div>" ).testWidget().testWidget( "widget" )[0] );
});
+test( ".instance()", function() {
+ expect( 2 );
+ var div,
+ _test = function() {};
+
+ $.widget( "ui.testWidget", {
+ _create: function() {},
+ _test: _test
+ });
+
+ div = $( "<div>" );
+ equal( div.testWidget( "instance" ), undefined );
@jzaefferer
jQuery Foundation member
jzaefferer added a note Mar 18, 2013

Usually calling methods on uninitialized widgets throws an error. Do we want to make the instance the exception, to check if a widget is initialized or not?

@scottgonzalez
jQuery Foundation member

Good point, we should just move the check for the instance method below the check for uninitialized widgets. Doesn't even require writing any additional code :-)

@gnarf
jQuery Foundation member
gnarf added a note Mar 18, 2013

This test case actually ensures that it's undefined not an exception. You don't want .widget( "instance" ) to throw IMO, or it's useless to be able to check to see if it has been initialized, and you'll still need to dig into data...

@scottgonzalez
jQuery Foundation member

I think I'm ok with this. @jzaefferer?

@jzaefferer
jQuery Foundation member
jzaefferer added a note Mar 19, 2013

Do we want to make the instance the exception, to check if a widget is initialized or not?

That's what I was asking about, so yeah, that makes sense. Afterwards we should make sure that we replace every instance of using data with the new instance method, for whatever purpose its used. And probably add some additional documentation, if we don't yet explicitly document how to retrieve the instance.

@scottgonzalez
jQuery Foundation member

Afterwards we should make sure that we replace every instance of using data with the new instance method, for whatever purpose its used.

This PR already does that.

And probably add some additional documentation, if we don't yet explicitly document how to retrieve the instance.

jquery/api.jqueryui.com#77

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jzaefferer
jQuery Foundation member

Reviewed. Found one broken demo and a very basic issue, see comments above. Otherwise looking good.

@jzaefferer
jQuery Foundation member

Replaced by #939 with a branch within this repo.

@jzaefferer jzaefferer closed this Mar 19, 2013
@gnarf gnarf deleted the gnarf:ticket-9030 branch Mar 19, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.