Opened 12 years ago
Closed 12 years ago
#9290 closed enhancement (fixed)
Suggest adding .length property to WidgetSet instances
Reported by: | dante | Owned by: | dante |
---|---|---|---|
Priority: | high | Milestone: | 1.4 |
Component: | Dijit | Version: | 1.3.0 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
Because WidgetSet doesn't return a real (or subclassed) array like NodeList, it is overly verbose to do the most simple of tasks: determine if there are any matches.
eg:
var sublist = dijit.registry.filter(function(w){ return w.value == someinput.value });
to determine if there are any instances in the sublist
new WidgetSet, you'd have to make a new variable, run .forEach and increment the variable, then check that.
var count = 0; var sublist = (...).forEach(function(w){ count++ }); if(count){ ... }
Seems convoluted, but by maintaining a .length member on the instance, the above can be reduced to:
if(dijit.registry.filter(function(w){ return w.value == input.value }).length){ // we pass the 'duplicate value' test! }
Attached is an ubersimple changeset adding in the functionality. By utilizing the existing .add and .remove api's (the only way to manipulate the widgetSet) the task is trivial.
Will update campus docs to include addition pending approval.
Attachments (2)
Change History (12)
Changed 12 years ago by
Attachment: | length.patch added |
---|
comment:1 Changed 12 years ago by
Owner: | changed from bill to dante |
---|
comment:2 Changed 12 years ago by
PS: although it would be better (at least for your example) to just have some() and every() methods.
comment:5 Changed 12 years ago by
Milestone: | tbd → 1.4 |
---|---|
Status: | new → assigned |
will add .every and .some and tests, and commit. Thanks for the review @bill
comment:6 Changed 12 years ago by
I have a minor improvement suggestion:
remove: function(/*String*/ id){ // Remove a widget from this WidgetSet. Does not destroy the widget; simply // removes the reference. if(this._hash[id]){ delete this._hash[id]; this.length = Math.max(0, this.length - 1); } },
comment:7 Changed 12 years ago by
if we add the if(this._hash[id]) the Math.max() is not required (it will never decrement unless the id is found) ... seems leaving out the if() check would be adequate in this case, but yes, alex had suggested max(0, --length) so it can never reach -1
comment:8 Changed 12 years ago by
I was trying to prevent a situation where calling remove() with an id of a widget that was already removed would decrement length. If there's no if(), it's possible to decrease length w/o removing a widget by making multiple remove() calls with the same id parameter.
comment:10 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Sure, sounds fine, go for it. I guess there should be a unit test too though. See also #4122.