Opened 12 years ago

Closed 10 years ago

Last modified 10 years ago

#4824 closed enhancement (fixed)

_Animation is exposed via NodeList animations and requires users to explicitly operate on it

Reported by: ptwobrussell Owned by: dante
Priority: high Milestone: 1.4
Component: Core Version: 1.0
Keywords: Cc: ptwobrussell@…
Blocked By: Blocking:

Description (last modified by Dustin Machi)

See below. The basic issues is a question of whether or not _Animation should lose the underscore and be made public or whether it's an exception to the normal rule about operating on privates (and thus would warrant a documentation update saying as much).

[ ] phiggins_: _Animation is like a generic class to hold all the .play()/etc and props [10:53pm] phiggins_: but it could be public

[10:54pm] phiggins_: and might should be as you can make a simple _Line and make an _Animation out of anything

[10:54pm] phiggins_: but making something public means

[10:54pm] phiggins_: that you have to keep the api the same and all that

[10:55pm] phiggins_: and ensure documentation yadda yadda

[10:55pm] ptwobrussell: phiggins_ : so 1) should i log a ticket indicating that this is an inconsistency in the api (whatever the solution ends up being, who knows) and 2) for now, it is accurate to say that it is an exception to the rule b/c it's "almost public" so people can use it...but it's not necessarily "final" yet?

[10:57pm] phiggins_: yes _Animation is only private because it is. 1) yes. NodeList?, etc are all exposed, Animation is almost stable enough, though there are some (public) things that suffer flaws and 2) yes, _private things aren't unsafe necessarily, just not subject to api-locking rules until deemed public.

[10:57pm] phiggins_: afaik

[10:58pm] phiggins_: but it does suffer flaws

[10:58pm] phiggins_: and a huge underlying _Animation change could happen and not effect the public stuff that uses it.

[10:59pm] ptwobrussell: phiggins_ : got it. i'll log a ticket to note the issue and report back in a sec.

[10:59pm] phiggins_: so it's almost easier to leave _that_ (_Animation) to the hackers, and make people use animateProperty()

[10:59pm] phiggins_: which is ultimately as "raw" _Animation as it gets

[11:02pm] ptwobrussell: phiggins_ : hrm. so maybe i shoudlnt' log a ticket then? if _Animation is considered "almost ready" and an exception to the normal rules about privates, then it's not really an issue, per se, assuming it eventually does get made public? so perhaps a simple documentation comment is all that is warranted?

[11:02pm] ptwobrussell: i.e. a comment that says as much

[11:04pm] phiggins_: maybe. i made a pass at documenting _Animation as the base "mixin" for all basic dojo.fx functions (and the api's for dojox.fx and base dojo fx lke _fade and animateProperty) because it applies to them all. eg: easing: curve: line: delay: repeat:

[11:04pm] phiggins_: so maybe to either dojo-book-document that fact about "Animations"

[11:04pm] phiggins_: or in line doc

Attachments (1)

publicAnim.patch (40.4 KB) - added by dante 10 years ago.
here she be

Download all attachments as: .zip

Change History (8)

comment:1 Changed 12 years ago by dante

Milestone: 1.01.1
Owner: changed from dante to alex

comment:2 Changed 11 years ago by dante

Milestone: 1.11.2

this would be a big documentation / api change for 1.1. punting for now.

comment:3 Changed 11 years ago by Dustin Machi

Description: modified (diff)
Type: defectenhancement

comment:4 Changed 11 years ago by dante

Milestone: 1.21.3
Owner: changed from alex to dante

feel free to take this back, I think we can do this for 1.3 provided we ensure all the docs are in place. inline docs are well in place, and the class is stable, the api is sane ...

comment:5 Changed 11 years ago by dante

Milestone: 1.31.4
Priority: normalhigh
Status: newassigned

K, Animation docs are in place more or less, so it would be easy to update these all in one pass. Going to punt to 1.4 and force this in in that timeframe and double check accuracy of all related docs.

Changed 10 years ago by dante

Attachment: publicAnim.patch added

here she be

comment:6 Changed 10 years ago by dante

Resolution: fixed
Status: assignedclosed

fixed in [17249], [17250], and [17251]

comment:7 Changed 10 years ago by Adam Peller

(In [17309]) !strict Fix typo. Refs #4824

Note: See TracTickets for help on using tickets.