Documenting 1.11's instance method for all widgets. #114

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
5 participants
@tjvantoll
Member

tjvantoll commented Mar 20, 2013

No description provided.

includes/widget-method-instance.xml
+<?xml version="1.0"?>
+<method name="instance">
+ <desc>
+ Retrieves the <placeholder name="name"/>'s instance object.

This comment has been minimized.

Show comment Hide comment
@jzaefferer

jzaefferer Mar 20, 2013

Owner

This should mention that it also works when the widget isn't initialized, unlike all other methods. It'll return undefined though (right?).

@jzaefferer

jzaefferer Mar 20, 2013

Owner

This should mention that it also works when the widget isn't initialized, unlike all other methods. It'll return undefined though (right?).

This comment has been minimized.

Show comment Hide comment
@tjvantoll

tjvantoll Mar 20, 2013

Member

Correct. And actually that's a much better way to detect whether an element is a widget than going through :data. I'll update this tonight.

@tjvantoll

tjvantoll Mar 20, 2013

Member

Correct. And actually that's a much better way to detect whether an element is a widget than going through :data. I'll update this tonight.

This comment has been minimized.

Show comment Hide comment
@scottgonzalez

scottgonzalez Mar 20, 2013

Owner

Just to make sure everyone understands: .foo( "instance" ) can't be used if the foo plugin hasn't been loaded, but :data(ns-foo) will always work. .foo( "instance" ) is preferred if you know the plugin exists.

@scottgonzalez

scottgonzalez Mar 20, 2013

Owner

Just to make sure everyone understands: .foo( "instance" ) can't be used if the foo plugin hasn't been loaded, but :data(ns-foo) will always work. .foo( "instance" ) is preferred if you know the plugin exists.

@tjvantoll

This comment has been minimized.

Show comment Hide comment
@tjvantoll

tjvantoll Mar 22, 2013

Member

Updated.

I think this now sufficiently covers #77 as well. @scottgonzalez @jzaefferer Let me know if this addresses your comments.

Member

tjvantoll commented Mar 22, 2013

Updated.

I think this now sufficiently covers #77 as well. @scottgonzalez @jzaefferer Let me know if this addresses your comments.

@jzaefferer

This comment has been minimized.

Show comment Hide comment
@jzaefferer

jzaefferer Mar 22, 2013

Owner

This addresses my comment, but not the one from Scott, since there's no mention of the method being unvailable if the plugin isn't loaded.

Owner

jzaefferer commented Mar 22, 2013

This addresses my comment, but not the one from Scott, since there's no mention of the method being unvailable if the plugin isn't loaded.

@tjvantoll

This comment has been minimized.

Show comment Hide comment
@tjvantoll

tjvantoll Mar 22, 2013

Member

Well, it is on the instance method docs for each widget, Unlike other widget methods, instance is safe to call on any element after the plugin has loaded. (https://github.com/jquery/api.jqueryui.com/pull/114/files#L16R4).

But it does look like the instance section on jQuery.widget doesn't mention that.

Member

tjvantoll commented Mar 22, 2013

Well, it is on the instance method docs for each widget, Unlike other widget methods, instance is safe to call on any element after the plugin has loaded. (https://github.com/jquery/api.jqueryui.com/pull/114/files#L16R4).

But it does look like the instance section on jQuery.widget doesn't mention that.

@jzaefferer

This comment has been minimized.

Show comment Hide comment
@jzaefferer

jzaefferer Mar 22, 2013

Owner

Okay, I see. I missed that even when specifically looking for it. Let's make that more explicit? For example, put that part on its own line. The instance method description doesn't have to be a single line.

Owner

jzaefferer commented Mar 22, 2013

Okay, I see. I missed that even when specifically looking for it. Let's make that more explicit? For example, put that part on its own line. The instance method description doesn't have to be a single line.

entries/jQuery.widget.xml
@@ -87,13 +87,19 @@
<h3>Instance</h3>
- <p>The widget's instance is stored using <a href="http://api.jquery.com/jQuery.data/"><code>jQuery.data()</code></a> with the widget's full name as the key. Therefore, you can use the following to retrieve the progressbar widget's instance object from the element.</p>
+ <p>The widget's instance can be retrieved from a given element using the <a href="#method-instance"><code>instance</code> method</a>.</p>

This comment has been minimized.

Show comment Hide comment
@scottgonzalez

scottgonzalez Mar 22, 2013

Owner

<a href="#method-instance"><code>instance()</code></a> method for consistency with other links.

Though I think we should do a pass to include the word option/method/event in the link.

@scottgonzalez

scottgonzalez Mar 22, 2013

Owner

<a href="#method-instance"><code>instance()</code></a> method for consistency with other links.

Though I think we should do a pass to include the word option/method/event in the link.

entries/jQuery.widget.xml
</code></pre>
- <p>Whether an element has a given widget bound to it can be determined using the <a href="/data-selector"><code>:data</code></a> selector.</p>
+ <p>If the <code>instance</code>method is called on an element that is not associated with the widget, <code>undefined</code> is returned.</p>

This comment has been minimized.

Show comment Hide comment
@scottgonzalez

scottgonzalez Mar 22, 2013

Owner

<code>instance()</code> method

parens and space

@scottgonzalez

scottgonzalez Mar 22, 2013

Owner

<code>instance()</code> method

parens and space

entries/jQuery.widget.xml
+ <p>If the <code>instance</code>method is called on an element that is not associated with the widget, <code>undefined</code> is returned.</p>
+
+ <pre><code>
+ $( "#not-a-progressbar" ).progressbar( "instance" ); // undefined

This comment has been minimized.

Show comment Hide comment
@scottgonzalez

scottgonzalez Mar 22, 2013

Owner

@kswedberg Do we ever put comments on the same line in docs? I'm assuming we don't since it goes against our style guide, but I know a lot of projects write docs like this. How do you normally show this in api.jquery.com?

@scottgonzalez

scottgonzalez Mar 22, 2013

Owner

@kswedberg Do we ever put comments on the same line in docs? I'm assuming we don't since it goes against our style guide, but I know a lot of projects write docs like this. How do you normally show this in api.jquery.com?

This comment has been minimized.

Show comment Hide comment
@kswedberg

kswedberg Mar 22, 2013

Member

We try to avoid same-line comments, but this is one scenario in which I haven't found a satisfactory alternative. I'll throw two options out there, but I don't know if they're any better than the EOL comment.

a. Comment on previous line, using a colon.

// Returns undefined: 
$( "#not-a-progressbar" ).progressbar( "instance" );

b. Comment on next line, being a bit more explicit.

var notProgress = $( "#not-a-progressbar" ).progressbar( "instance" );
// notProgress === undefined
@kswedberg

kswedberg Mar 22, 2013

Member

We try to avoid same-line comments, but this is one scenario in which I haven't found a satisfactory alternative. I'll throw two options out there, but I don't know if they're any better than the EOL comment.

a. Comment on previous line, using a colon.

// Returns undefined: 
$( "#not-a-progressbar" ).progressbar( "instance" );

b. Comment on next line, being a bit more explicit.

var notProgress = $( "#not-a-progressbar" ).progressbar( "instance" );
// notProgress === undefined

This comment has been minimized.

Show comment Hide comment
@scottgonzalez

scottgonzalez Mar 22, 2013

Owner

@dmethvin @rwldrn thoughts?

@scottgonzalez

scottgonzalez Mar 22, 2013

Owner

@dmethvin @rwldrn thoughts?

This comment has been minimized.

Show comment Hide comment
@dmethvin

dmethvin Mar 22, 2013

Owner

A same-line comment seems right here.

@dmethvin

dmethvin Mar 22, 2013

Owner

A same-line comment seems right here.

This comment has been minimized.

Show comment Hide comment
@tjvantoll

tjvantoll Mar 23, 2013

Member

I can understand the reasoning for avoiding same line docs in the code base, but for succinct code examples I think they make sense.

@tjvantoll

tjvantoll Mar 23, 2013

Member

I can understand the reasoning for avoiding same line docs in the code base, but for succinct code examples I think they make sense.

entries/jQuery.widget.xml
+ $( "#not-a-progressbar" ).progressbar( "instance" ); // undefined
+ </code></pre>
+
+ <p>The instance is stored using <a href="http://api.jquery.com/jQuery.data/"><code>jQuery.data()</code></a> with the widget's full name as the key. Therefore, you can use the <a href="/data-selector"><code>:data</code></a> selector to determine whether an element has a given widget bound to it.</p>

This comment has been minimized.

Show comment Hide comment
@scottgonzalez

scottgonzalez Mar 22, 2013

Owner

This is the perfect place to put the note about :data working when instance() will not.

@scottgonzalez

scottgonzalez Mar 22, 2013

Owner

This is the perfect place to put the note about :data working when instance() will not.

includes/widget-method-instance.xml
+<?xml version="1.0"?>
+<method name="instance" return="Object">
+ <desc>
+ Retrieves the <placeholder name="name"/>'s instance object. Unlike other widget methods, <code>instance</code> is safe to call on any element after the <placeholder name="name"/> plugin has loaded. If the element does not have an associated instance, <code>undefined</code> is returned.

This comment has been minimized.

Show comment Hide comment
@scottgonzalez

scottgonzalez Mar 22, 2013

Owner

<code>instance()</code>

@scottgonzalez

scottgonzalez Mar 22, 2013

Owner

<code>instance()</code>

@tjvantoll

This comment has been minimized.

Show comment Hide comment
@tjvantoll

tjvantoll Mar 23, 2013

Member

@scottgonzalez @jzaefferer Updates made per your comments.

Member

tjvantoll commented Mar 23, 2013

@scottgonzalez @jzaefferer Updates made per your comments.

@jzaefferer

This comment has been minimized.

Show comment Hide comment
@jzaefferer

jzaefferer Mar 24, 2013

Owner

Alright, looks good to me.

Owner

jzaefferer commented Mar 24, 2013

Alright, looks good to me.

entries/jQuery.widget.xml
+ $( "#not-a-progressbar" ).progressbar( "instance" ); // undefined
+ </code></pre>
+
+ <p>The instance is stored using <a href="http://api.jquery.com/jQuery.data/"><code>jQuery.data()</code></a> with the widget's full name as the key. Therefore, the <a href="/data-selector"><code>:data</code></a> selector can also determine whether an element has a given widget bound to it.</p>

This comment has been minimized.

Show comment Hide comment
@scottgonzalez

scottgonzalez Mar 25, 2013

Owner

/data-selector/ (trailing slash)

@scottgonzalez

scottgonzalez Mar 25, 2013

Owner

/data-selector/ (trailing slash)

@scottgonzalez

This comment has been minimized.

Show comment Hide comment
@scottgonzalez

scottgonzalez Mar 25, 2013

Owner

Just one minor change to make. This look good to land.

Owner

scottgonzalez commented Mar 25, 2013

Just one minor change to make. This look good to land.

@tjvantoll

This comment has been minimized.

Show comment Hide comment
@tjvantoll

tjvantoll Mar 26, 2013

Member

Landed in 06d80fe & cde5d57.

Member

tjvantoll commented Mar 26, 2013

Landed in 06d80fe & cde5d57.

@tjvantoll tjvantoll closed this Mar 26, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment