#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:
- Executes the connected function in the scope of the widget
- 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)
Change History (11)
Changed 12 years ago by
Attachment: | 8227_widgetSubscribe.patch added |
---|
comment:1 Changed 12 years ago by
Summary: | dijit._Widget: add in a subscribe function → [patch][cla] dijit._Widget: add in a subscribe function |
---|
comment:2 Changed 12 years ago by
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 follow-up: 4 Changed 12 years ago by
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 Changed 12 years ago by
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 12 years ago by
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 12 years ago by
Milestone: | 1.3 → future |
---|
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 12 years ago by
Milestone: | future → 1.4 |
---|
comment:8 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Fairly simple patch which implements this functionality