Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#8227 closed enhancement (fixed)

[patch][cla] dijit._Widget: add in a subscribe function

Reported by: Nathan Toone Owned by: Nathan Toone
Priority: high Milestone: 1.4
Component: Dijit Version: 1.2.1
Keywords: Cc:
Blocked By: Blocking:

Description

dijit._Widget has a nice convenience function that maps dojo.connect. It does 2 things:

  1. Executes the connected function in the scope of the widget
  2. Cleans up any still-connected functions when destroy() is called

It would be helpful to have a similar convenience function in dijit._Widget for subscribe (and unsubscribe). It would behave very similarly to connect (and disconnect)

Attachments (1)

8227_widgetSubscribe.patch (2.4 KB) - added by Nathan Toone 11 years ago.
Fairly simple patch which implements this functionality

Download all attachments as: .zip

Change History (11)

Changed 11 years ago by Nathan Toone

Attachment: 8227_widgetSubscribe.patch added

Fairly simple patch which implements this functionality

comment:1 Changed 11 years ago by Nathan Toone

Summary: dijit._Widget: add in a subscribe function[patch][cla] dijit._Widget: add in a subscribe function

comment:2 Changed 11 years ago by dante

afaik .connect() only does the array-of-arrays thing because of the extra possible a11y handling needing to be done, so the extra forEach'ing in the teardown doesn't seem necessary? maybe I'm wrong though.

also, we gain very little by the way the subscribe function is written (wrt to 'var' usage)

var dsb = dojo.partial(dojo.subscribe, topic),
    handle = dsb(this, method);
this._subscribes.push(handle); 
return handle;

would work right? again, the handles array there is unnecessary as nothing special needs to happen wrt to subscribe (see .connect() code)

unsubscribe would be shorter too:

dojo.some(this._subscribes, function(h,i){ 
    if(h == handle){
      dojo.subsubscribe(h);
      delete this._subscribes[i];
      return true;
    }
}, this); // actually not sure if some scopes 'this' like this

but it would need tests as well, and reference docs in both the using and writing widget guides.

comment:3 Changed 11 years ago by Nathan Toone

I thought that the array of arrays was also helpful by not accidentally passing something to dojo.disconnect that should be passed to widget.disconnect....that was the main reason for my doing it the same way (that and I just copied and pasted).

the unsubscribe function wouldn't quite work, I don't think - since it's just leaving empty array elements around - instead of splicing the array together. But we could do something like this:

subscribe: function(topic, method){
    var handle = dojo.subscribe(topic, this, method);
    this._subscribes.push(handle);
    return handle;
},
unsubscribe: function(handle){
    this._subscribes = dojo.filter(this._subscribes, function(h){
        if(h == handle){
            dojo.unsubscribe(h);
            return false;
        }
        return true;
    }, this);
}

And yes - I was actually going to ask where the best place to doc this on docs.dojocampus.org would be. There isn't a page for dijit._Widget - but I don't know that there really should be one...since people generally don't instantiate it directly. Do we have documentation for how to write your own widget? If so, it would probably belong there, in my opinion.

comment:4 in reply to:  3 Changed 11 years ago by dante

Replying to toonetown:

And yes - I was actually going to ask where the best place to doc this on docs.dojocampus.org would be. There isn't a page for dijit._Widget - but I don't know that there really should be one...since people generally don't instantiate it directly. Do we have documentation for how to write your own widget? If so, it would probably belong there, in my opinion.

http://docs.dojocampus.org/quickstart/writingWidgets

but your inline api-doc example shows it in the context of user-code, not internal widgeting code. it should likely show both examples:

// writing:
postCreate:function(){
    this.subscribe(this.id+"-over", "_handler")
},
_handler: function(msg){
    dojo.addClass(this.domNode, "over"0;
}

and

var b = new dijit.Dialog(...);
b.subscribe("/open/it/up", "show"); // calls b.show() or this.show() rather

and maybe showing how to use in context of self:

var b = new dijit.form.Button(...);
b.subscribe("/go/away", function(){ this.destroy(); });
dojo.publish("/go/away");

So we definitely need better writing widget docs (though they are fairly good, just don't dive into the nuances and scope like this), but they are useful (as seen in examples) outside of the scope of writing. Maybe a page for 'functions in all widgets' ? or maybe a generic widget page explaining _what_ _Widget actually is, and how it applies, and cover all the connect/disconnect/destroy/etc functions inherent in all places.

would also be an excellent place to document this technique:

dojo.extend(dijit._Widget, { myCustomAttribute:"defaultSomething" });

and why it causes so much confusion in the API docs. (eg: why does a button have a region property?)

comment:5 Changed 11 years ago by bill

I don't mind adding this code, but let's be careful with the naming... Widget.connect() was actually a mistake. It sounds like a method widget users use to listen to events on the widget (onClick, onClick, etc.), but it's actually usually internally by widgets.... should have been called something else, not sure what, but something with an underscore to indicate a "protected" method (to use java terms).

This code also seems to be for use by widget developers; so, let's name it with an underscore, and hopefully some name that indicates that the widget is listening to some external topic rather than a user listening to some topic published by the widget (although no good names come to mind).

Oh, as Pete said the array-of-array things is just to handle onDijitClick, which is implemented internally w/2 dojo.connect()'s.

Oh, and as Nathan said, I'd like to use splice() rather than leaving holes in the array.

comment:6 Changed 11 years ago by dante

Milestone: 1.3future

that's too bad widget.connect was a mistake. I like it, and I like that the _Widget functions act "upon 'this' as their dojo.* counterparts" (eg: connect, placeAt). subscribe sounds nice, and would be great to be exposed in public for users, but it sounds like there is some confusion so punting for now.

comment:7 Changed 10 years ago by Nathan Toone

Milestone: future1.4

comment:8 Changed 10 years ago by Nathan Toone

Resolution: fixed
Status: newclosed

(In [17653]) Fixes #8227 - add in a subscribe/unsubscribe function set to dijit._Widget !strict

comment:9 Changed 10 years ago by Nathan Toone

(In [17698]) Refs #8227 improve performance by not using arrays where not needed in subscribe, and add in a test case !strict

comment:10 Changed 10 years ago by Nathan Toone

(In [17704]) Refs #8227 improve performance even more by not doing a hitch !strict

Note: See TracTickets for help on using tickets.