Opened 12 years ago

Closed 12 years ago

#4480 closed defect (fixed)

NodeList animations cause error

Reported by: simonjb Owned by: alex
Priority: high Milestone: 1.0
Component: Core Version: 0.9
Keywords: Cc: ptwobrussell@…
Blocked By: Blocking:

Description

The NodeList animations make a call to dojo.fx.combine(), which is not in _base. Calling one of the animations (fadeIn, fadeOut or animateProperty) results in an error: d.fx has no properties.

There is already a FIXME comment in the code but I thought a trac ticket might be useful too.

_anim: function(method, args){
	var anims = [];
	args = args||{};
	this.forEach(function(item){
		var tmpArgs = { node: item };
		d.mixin(tmpArgs, args);
		anims.push(d[method](tmpArgs));
	});
	// FIXME: combine isn't in Base!!
	return d.fx.combine(anims); // dojo._Animation
},

Change History (6)

comment:1 Changed 12 years ago by guest

Options to fix:

  1. move dojo.fx.combine to core, i.e. dojo.combine
  2. remove built-in animations from NodeList?
  3. copy dojo.fx.combine into dojo._base.NodeList?
  4. warn people that they must require dojo.fx in order to use NodeList?

Got time to work on this. As a noob, need help choosing best solution. Fixing this is necessary to get all tests working, see #3121 -morphatic

comment:2 Changed 12 years ago by ptwobrussell

Cc: ptwobrussell@… added

My thoughts are that a dojo.require("dojo.fx") should be added to the _anim function as a temporary solution so that the animations actually work. anyone who realizes that all they have to do to fix this problem is do dojo.require("dojo.fx") call before trying to run the animations will do it anyhow. Adding this in as a temp fix 1) actually makes the animations work, 2) causes the tests to pass, and 3) doesn't require people to "work around" the problem.

OTOH, eventually, it would have to be determined if combine should be shuffled around so that Base doesn't have a non-Base dependency. In the meantime though, adding the temporary dojo.require statement wouldn't make it any more broken than it already is.

comment:3 Changed 12 years ago by alex

(In [11145]) adding first extension file for dojo.NodeList?. Refs #4480

comment:4 Changed 12 years ago by alex

(In [11146]) making the constructor smaller ('cause we can), adding slice/splice, and moving animations out to dojo.NodeList?-ext. Refs #4480. Refs #4205. Fixes #4822

comment:5 Changed 12 years ago by Adam Peller

(In [11155]) Rename NodeList? extension to reflect fx additions. Refs #4480

comment:6 Changed 12 years ago by alex

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.