panTween needs to be nullified

[pgiraud]

Eric previously proposed we don't recreate a new Tween object each time we call panTo. But, this means that, with the current if conditions in moveTo(), panTween.stop() will be called even if not required. Attached patch fixes this by nullifying the panTween property.

One other solution would be to have a 'isPlaying' boolean property in the tween class and only call stop if isPlaying is true.

[pgiraud]

I can easily provide a new patch if you guys prefer the second solution.

[crschmidt]

I think that this is better, but still not perfect, because we don't actually set the panTween to 'null' when we're done panning, only the next time we setCenter, which means that there will always be one extra panTween.done called... Seems like we should *also* nullify it in panTween.done (in moveTo)?

[pgiraud]

This new patch adds a new 'playing' property to the Tween class. A call to stop doesn't do anything if tween isn't playing. No change needed in Map.js with this solution.

Patch also includes completed tests and the missing 'panTween' property declare statement in Map.

Please review.

[pgiraud]

Forgot to mention that tests pass on FF2 and IE6 (if we don't consider errors in Layer_MapGuide).

[ahocevar]

Pierre: I'm totally in favour of the second patch. Thanks for addressing this.

The patch looks perfect, please commit!

[pgiraud]

I would like Christopher's opinion on that before I commit.

[crschmidt]

Why don't we just nullify the panTween in the function bound to done? Wouldn't that take care of it?

I'm happy with either approach, really. Just curious.

[pgiraud]

Replying to crschmidt:

Why don't we just nullify the panTween in the function bound to done? Wouldn't that take care of it?

The new 'playing' approach doesn't require any change in the Map.js code. I have set the 'playing' to Property (non API) because the code in Map.js doesn't have to know anything about it.

If I choose to nullify the panTween I would have to do it in several location in the code. I'm not sure I won't forget one.

[crschmidt]

Okay, that's fair enough. Essentially, this prevents us from shooting ourselves in the foot slightly more than the other way around.

With that in mind, and Andreas's review, I'm happy if you are.

[pgiraud]

(In [6387]) add a new 'playing' property to the Tween class so that a call to stop doesn't do anything if animation is already finished, r=ahocevar,crschmidt (Closes #1392)

[tschaub]

(In [6388]) Invoking the trivial change clause, I'm adding a semicolon. (see #1392)