tween (animation) in OpenLayers

Paul (or other folks), I would be very happy if you can have a look at this. Chris, in your opinion, how do I have to deal with licence ?

I think this is great stuff. The Tween class should be pretty easy to integrate into the animated zooming, reduce the code a bit, and provide some interesting options.

For this specific patch, I have a couple of comments:

• there seems to be an extra button left over in the example.

• I am concerned about the panTo method not being 're-entrant' ... perhaps panTo and setCenter should cancel any existing tween before doing their jobs?

Replying to pspencer:

I think this is great stuff. The Tween class should be pretty easy to integrate into the animated zooming, reduce the code a bit, and provide some interesting options. For this specific patch, I have a couple of comments: * there seems to be an extra button left over in the example.

Good catch, I'll correct this and probably add an new input to let user choose the duration of the animation.

* I am concerned about the panTo method not being 're-entrant' ... perhaps panTo and setCenter should cancel any existing tween before doing their jobs?

I totaly agree with that, but my mistake, the panTo shouldn't have been in this patch. I would like this to settle in #110 instead.

New patch to follow.

same patch with minor fixes and without the animated panning code

tests pass under FF almost all tests pass under IE6 : test_OSM doesn't but it doesn't with trunk for me either

Replying to pgiraud:

tests pass under FF almost all tests pass under IE6 : test_OSM doesn't but it doesn't with trunk for me either

Pierre,

The code looks good to me!

A few comments:

• The initialize method doesn't have the usual ND comment indicating that it is the constructor.
• In the tween.html example you may want to call stop() on the previous tween object when changing the tween.
• By the way, still in the tween.html example, why do you need to create a new tween object to change the easing? Reading your code, I think your could just do tween.easing = OpenLayers.Easing[transition][easing], couldn't you?
• Even though I think the tween.html example is useful, I'm not sure it should be in the examples/ directory. To me the things in examples/ show the end-user how to use OpenLayers. The tween code doesn't belong the OpenLayers API.
• The functions of the Easing objects are marked APIFunction. Do we really want these functions to be part of the OpenLayers API? (to me: no)

Thanks very much for this great review. I think I should have checked the code before I set this ticket's state to review.

* The initialize method doesn't have the usual ND comment indicating that it is the constructor.

Ok, and I completed/modified some other ND docs. Some properties are now marked as API.

* In the tween.html example you may want to call stop() on the previous tween object when changing the tween.

It looks like it a good choice. I don't think this is really needed.

* By the way, still in the tween.html example, why do you need to create a new tween object to change the easing? Reading your code, I think your could just do tween.easing = OpenLayers.Easing[transition][easing], couldn't you?

You seem to be right. Changed.

* Even though I think the tween.html example is useful, I'm not sure it should be in the examples/ directory. To me the things in examples/ show the end-user how to use OpenLayers. The tween code doesn't belong the OpenLayers API.

Agreed. I moved the tween.html example to tests/manual. Do you think this is a better idea ?

* The functions of the Easing objects are marked APIFunction. Do we really want these functions to be part of the OpenLayers API? (to me: no)

I mixed API/ non API with private/public, my bad. Fixed.

Everything is fixed in the upcoming patch. I think, this is now ready for a last review.

With licensing bits

Pierre:

I've reviewed this, and I'm ready for it to go to trunk. I've uploaded a patch which is exactly the same as yours, only with licensing bits added; please use this one, and go ahead to commit when ready.

(In [6032]) Add tweening to sandbox (See #1308)

fixed in r6097