Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#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 6 years ago by dylan

Milestone: tbd1.9.4
Owner: set to bill
Priority: undecidedhigh
Status: newassigned

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.

comment:2 Changed 6 years ago by bill

@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 6 years ago by Bill Keese <bill@…>

Resolution: fixed
Status: assignedclosed

In ef21a63853d1521248869a3b05b0d7493558419e/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:4 Changed 6 years ago by Bill Keese <bill@…>

In 3168e79e2191ac144925f1ba0eeb117308bdd5ae/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 
Note: See TracTickets for help on using tickets.