Bug 110132 – Add animation class for new calendar picker

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Keishi Hattori]

Add animation related classes for the new calendar picker

[Keishi Hattori]

Created attachment 188910 [details]
Patch

[Kent Tamura]

Comment on attachment 188910 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=188910&action=review

> Source/WebCore/Resources/pagepopups/calendarPicker.js:782
> +    this.dispatchEvent("animationFrameWillFinish");

If the name "animationFrameWillFinish" is used by other classes, this class should provide a constant symbol for it.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:797
> + */

need @override

> Source/WebCore/Resources/pagepopups/calendarPicker.js:807
> + */

Ditto.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:831
> +    this.id = Animator.lastId++;
> +    this.from = 0;
> +    this.to = 0;
> +    this._delta = 0;
> +    this.duration = 100;
> +    this.step = null;
> +    this.lastStepTime = null;
> +    this.progress = 0.0;
> +    this.timingFunction = AnimationTimingFunction.Linear;

Should these members be public?  If they should not be updated by other objects, they should be private and should provide accessor functions.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:832
> +};

; is not needed.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:860
> +    this.dispatchEvent("didAnimationStart");

If the name "didAnimationStart" is used by other classes, this class should provide a constant symbol for it.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:870
> +    this.dispatchEvent("didAnimationStop", this);

If the name "didAnimationStop" is used by other classes, this class should provide a constant symbol for it.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:880
> +        return this.stop();

confusing code. should be
  this.stop();
  return;

[Keishi Hattori]

Created attachment 189491 [details]
Patch

[Kent Tamura]

Comment on attachment 189491 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=189491&action=review

> Source/WebCore/ChangeLog:14
> +        (AnimationTimingFunction.Linear): Parameter t should be a number between 0 and 1.

Such comment should be in the code, or the code should have console.assert.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:841
> +Animator.lastId = 0;

I guess this should be private.

[Keishi Hattori]

Created attachment 189516 [details]
Patch

[WebKit Review Bot]

Comment on attachment 189516 [details]
Patch

Clearing flags on attachment: 189516

Committed r143598: <http://trac.webkit.org/changeset/143598>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.