Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#3338 closed defect (fixed)

dojo.fadeOut and dojo.fadeIn have incorrect documentation and incorrect internal implementation for array of DOM nodes

Reported by: bradneuberg Owned by: Bryan Forbes
Priority: high Milestone: 0.9beta
Component: lfx Version: 0.9
Keywords: Cc:
Blocked By: Blocking:

Description

I am porting the Dojo Storage test page, at dojox/storage/tests/test_storage.html to the new Dojo 0.9 system. I use the following call:

dojo.fadeOut({nodes: status, duration: 2000}).play()

which results in the following exception:

Object cannot be created in this context" code: "9 undefined

The problem comes down to this call in the Dojo FX code:

this.fire("beforeBegin");

in the method dojo._Animation.play(). The exact issue is inside the fire method:

if(this[evt]){

this[evt].apply(this, args
[]);

}

Where we have an evt, "beforeBegin", but 'args' is undefined. I believe the apply method calls through to a listener, but we never defined a beforeBegin callback in my Test Storage page.

Attachments (1)

fx-inline-doc.patch (4.9 KB) - added by guest 13 years ago.
patch to mirror _current_ behavior of _base fx and dojo.fx with inline docs

Download all attachments as: .zip

Change History (6)

comment:1 Changed 13 years ago by bradneuberg

Summary: dojo.fadeOut throws exceptiondojo.fadeOut and dojo.fadeIn have incorrect documentation and incorrect internal implementation for array of DOM nodes

The actual source of this bug is either an error in documentation or an error in implementation. In the method dojo.fadeOut, the documentation says the following for the 'nodes' property that can be passed in:

dojo.fadeOut = function(/*Object*/ args){

summary: Returns an animation that will fade "nodes" from its current opacity to fully transparent. nodes: An array of DOMNodes or one DOMNode. duration: Duration of the animation in milliseconds. easing: An easing function. return dojo._fade(dojo.mixin({ end: 0 }, args)); dojo._Animation

}

It says to pass in 'nodes' as an individual DOM node or an array of DOM Nodes. However, if we dig down into the dojo._fade method, we see the following:

args.node = dojo.byId(args.node);

var fArgs = dojo.mixin({ properties: {} }, args); var props = fArgs.properties.opacity = {}; props.start = (typeof fArgs.start == "undefined") ?

function(){ return Number(dojo.style(fArgs.node, "opacity")); } : fArgs.start;

props.end = fArgs.end;

var anim = dojo.animateProperty(fArgs); dojo.connect(anim, "beforeBegin", null, function(){

_makeFadeable(fArgs.node);

});

Notice that it is 'args.node' not 'args.nodes'; this also holds true for the dojo.connect on beforeBegin, which uses 'fArgs.node' not 'fArgs.nodes'.

So, internally the system looks like it knows how to work with just one DOM Node, and the way you pass things in is with the argument 'node' not 'nodes' plural. I don't want to just change the word to be plural, because I also suspect that the internal machinery doesn't know how to deal with an array, which needs to be baked in.

Note that the dojo.fadeIn bugs suffers from the same issues.

Changing title of bug.

comment:2 Changed 13 years ago by Adam Peller

Component: Corelfx
Milestone: 0.9beta
Owner: changed from anonymous to Bryan Forbes

Changed 13 years ago by guest

Attachment: fx-inline-doc.patch added

patch to mirror _current_ behavior of _base fx and dojo.fx with inline docs

comment:3 in reply to:  2 Changed 13 years ago by guest

Replying to peller:

actually, all the FX functions still refer to Nodes but use a unified args object for mixins.

patch submitted [cla, phiggins] to fix inline docs.

comment:4 Changed 13 years ago by Adam Peller

Resolution: fixed
Status: newclosed

(In [9165]) Improve docs. Fixes #3338

comment:5 Changed 13 years ago by Adam Peller

p.s. thanks, kjamezz!

Note: See TracTickets for help on using tickets.