Opened 9 years ago

Closed 9 years ago

#11472 closed task (fixed)

Unnecessary code in AccordionContainer?

Reported by: xMartin Owned by: bill
Priority: low Milestone: 1.6
Component: Dijit Version: 1.5
Keywords: Cc:
Blocked By: Blocking:

Description

Line 179-185 in dijit/layout/AccordionContainer.js:

		_transition: function(/*dijit._Widget?*/newWidget, /*dijit._Widget?*/oldWidget, /*Boolean*/ animate){
			// Overrides StackContainer._transition() to provide sliding of title bars etc.

//TODO: should be able to replace this with calls to slideIn/slideOut
			if(this._inTransition){ return; }
			var animations = [];
			var paneHeight = this._verticalSpace;

Seems as if the variable paneHeight in the last line is never used and thus unnecessary and confusing. Or am I missing something?

Change History (5)

comment:1 Changed 9 years ago by bill

Milestone: tbd1.6
Owner: set to bill
Status: newassigned

Thanks for the catch, I'll remove the line.

comment:2 Changed 9 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [22541]) remove unused code, fixes #11472 !strict

comment:3 Changed 9 years ago by xMartin

Resolution: fixed
Status: closedreopened

So it appears that there may be more unnecessary lines here.

Please check line 198-199:

dojo.addClass(newContents, "dijitVisible");
dojo.removeClass(newContents, "dijitHidden");

Same addClass/removeClass statements are already called in _showChild() which is invoked a few lines above.

comment:4 Changed 9 years ago by bill

Good catch, plus looks like we can reduce further by calling _hideChild() too (with the added benefit that it sets child.selected to false (counteracting what _showChild() did).

comment:5 Changed 9 years ago by bill

Resolution: fixed
Status: reopenedclosed

(In [22618]) Further AccordionContainer code reductions, fixes #11472 again, !strict.

Note: See TracTickets for help on using tickets.