#17748 closed defect (fixed)
InlineEditBox with autosave=true doesn't destroy the save/cancel buttons...
Reported by: | overtune | Owned by: | bill |
---|---|---|---|
Priority: | high | Milestone: | 1.9.4 |
Component: | Dijit | Version: | 1.9.2 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
When creating an InlineEditBox? with autosave=true the save- and cancel-buttons a still registered as widgets after the InlineEditBox? is destroyed. The number of widgets in dijit.registry._hash keeps growing...
The problem seems to be on line 105-109 of InlineEditBox?.js:
if(this.inlineEditBox.autoSave){ // Remove the save/cancel buttons since saving is done by simply tabbing away or // selecting a value from the drop down list domConstruct.destroy(this.buttonContainer); }
This only seems to destroy the dom-elements, but the buttons seems to be created already and will still be left.
I'm quite new to Dojo, so I'm not shure what's the best solution. But explicitly destroing the buttons in the destroy function seems to work. Line 147:
destroy: function(){ this.saveButton.destroy(); this.cancelButton.destroy(); this.editWidget.destroy(true); // let the parent wrapper widget clean up the DOM this.inherited(arguments); },
This is my first ticket created here, so bear with me if there is something incorrect or inappropriate.
Change History (4)
comment:1 Changed 8 years ago by
Milestone: | tbd → 1.9.4 |
---|---|
Owner: | set to bill |
Priority: | undecided → high |
Status: | new → assigned |
comment:2 Changed 8 years ago by
@overtune - Thanks, indeed 105-109 of InlineEditBox?.js look problematic. Seems like that code should be destroying the underlying widgets too, before removing this.buttonContainer. Either with the code you suggested for destroy:
this.saveButton.destroy(); this.cancelButton.destroy();
or something fancy (i.e. more general) like:
array.forEach(registry.findWidgets(this.buttonContainer), function(w){ w.destroy(); }));
comment:3 Changed 8 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
There may be a cleaner way to clean this up, but the proposed patch is probably fine, as these aren't proper parent/child widgets, and this isn't something that this.own would easily clean up on the widget level since it's the destruction of a subset of the widget.