Bug 154738 – Web Inspector: Custom transition bezier curve editor preview should loop when not editing curve

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Timothy Hatcher]

Right now the only way I can see to preview the animation is to edit the curve or click on the animation itself.

It would be nice if it looped when editing was inactive. Clicking could toggle the animation looping?

<rdar://problem/24861563>

[Devin Rousso]

(In reply to comment #0)
> Right now the only way I can see to preview the animation is to edit the
> curve or click on the animation itself.

I may be misunderstanding what you mean by "animation itself", but clicking on the preview area will restart the animation (regardless of its current progress).

> It would be nice if it looped when editing was inactive. Clicking could
> toggle the animation looping?

So instead of clicking to restart, make clicking toggle looping? Why not just make it loop by default and allow clicking to restart?

[Timothy Hatcher]

(In reply to comment #1)
> (In reply to comment #0)
> > Right now the only way I can see to preview the animation is to edit the
> > curve or click on the animation itself.
> 
> I may be misunderstanding what you mean by "animation itself", but clicking
> on the preview area will restart the animation (regardless of its current
> progress).
> 
> > It would be nice if it looped when editing was inactive. Clicking could
> > toggle the animation looping?
> 
> So instead of clicking to restart, make clicking toggle looping? Why not
> just make it loop by default and allow clicking to restart?

Looping by default would likely be fine.

[Devin Rousso]

Created attachment 272391 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 272391 [details]
Patch

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

Yay! I wanted this the last time I used it.

> Source/WebInspectorUI/UserInterface/Views/BezierEditor.js:41
> -        this._bezierPreviewAnimationStyleText = "bezierPreview 2s 250ms forwards ";
> +        this._bezierPreviewAnimationStyleText = "bezierPreview 2.5s 250ms infinite ";

Is this member necessary?

It seems to be used here:

        this._bezierPreview.style.animation = this._bezierPreviewAnimationStyleText + this._bezier.toString();

But perhaps we can just use the CSS version and set the bezier.toString on the more specific property:

        this._bezierPreview.style.animationTimingFunction = this._bezier.toString();

Would this work? It would avoid the duplication, and better match the actual intention.

[WebKit Commit Bot]

Comment on attachment 272391 [details]
Patch

Clearing flags on attachment: 272391

Committed r197235: <http://trac.webkit.org/changeset/197235>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Devin Rousso]

(In reply to comment #4)
> Comment on attachment 272391 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=272391&action=review
> 
> Yay! I wanted this the last time I used it.
> 
> > Source/WebInspectorUI/UserInterface/Views/BezierEditor.js:41
> > -        this._bezierPreviewAnimationStyleText = "bezierPreview 2s 250ms forwards ";
> > +        this._bezierPreviewAnimationStyleText = "bezierPreview 2.5s 250ms infinite ";
> 
> Is this member necessary?
> 
> It seems to be used here:
> 
>         this._bezierPreview.style.animation =
> this._bezierPreviewAnimationStyleText + this._bezier.toString();
> 
> But perhaps we can just use the CSS version and set the bezier.toString on
> the more specific property:
> 
>         this._bezierPreview.style.animationTimingFunction =
> this._bezier.toString();
> 
> Would this work? It would avoid the duplication, and better match the actual
> intention.

Nice catch! Not really sure why I didn't do this in the first place :P
Addressed in <https://bugs.webkit.org/show_bug.cgi?id=154809>