Devtools Animations: Update timeline controls and UI

Devtools Animations: Update timeline controls and UI

This change updates the UI of the animation timeline and removes the
pause button. Instead, the pause button is combined into a single
control button to pause, play and replay the timeline.

BUG=492679, 447083
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201749

[samli, 2015-05-28 07:26:39]

samli@chromium.org changed reviewers:
+ maxwalker@chromium.org, pfeldman@chromium.org

[samli, 2015-05-28 07:26:40]

PTAL.

Screenshots: http://imgur.com/a/yIMZ2
UI changes:
 - add empty timeline message
 - uses normal play/pause icons and the animations replay icon
 - drag handles for the scrubber
 - input border for playback rate

Max: should we stick with the normal material icons for play/pause/replay?

[maxwalker, 2015-05-28 11:17:26]

> - add empty timeline message
I wasn't quite sure about the wording here. Any suggestions for improvements
appreciated! :) 

> drag handles for the scrubber
As the scrubber can now stay put at 0ms it's possible that it covers the control
button.
I prevented this problem in the designs by shifting the 0ms line some pixels to
the right (also adds some room to breathe on the left): https://goo.gl/jDU4Np 

> Max: should we stick with the normal material icons for play/pause/replay?
I modified the replay icon (added the play triangle), so that it looks more like
"play" and less like "undo".
You can find all three icons in the Animations assets folder.

[pfeldman, 2015-05-29 13:12:25]

Please split this into making the glyph image larger and changes to the
semantics.

[samli, 2015-06-01 01:32:11]

On 2015/05/29 at 13:12:25, pfeldman wrote:
> Please split this into making the glyph image larger and changes to the
semantics.

Done. PTAL

[samli, 2015-06-04 08:08:40]

ping

[maxwalker, 2015-06-05 08:27:17]

lgtm

[pfeldman, 2015-06-05 14:34:34]

https://codereview.chromium.org/1156323003/diff/60001/Source/devtools/front_e...
File Source/devtools/front_end/elements/AnimationControlPane.js (right):

https://codereview.chromium.org/1156323003/diff/60001/Source/devtools/front_e...
Source/devtools/front_end/elements/AnimationControlPane.js:144: if (event &&
this._animationTimeline)
When is event undefined?

https://codereview.chromium.org/1156323003/diff/60001/Source/devtools/front_e...
File Source/devtools/front_end/elements/AnimationTimeline.js (right):

https://codereview.chromium.org/1156323003/diff/60001/Source/devtools/front_e...
Source/devtools/front_end/elements/AnimationTimeline.js:163:
_playbackLabelInput: function(event)
I applied your change and I can now enter random text into that field. Please
test your changes thoroughly.

https://codereview.chromium.org/1156323003/diff/60001/Source/devtools/front_e...
Source/devtools/front_end/elements/AnimationTimeline.js:212:
this._controlButton.setTitle("Play timeline");
UIString please.

[samli, 2015-06-09 03:53:08]

https://codereview.chromium.org/1156323003/diff/60001/Source/devtools/front_e...
File Source/devtools/front_end/elements/AnimationControlPane.js (right):

https://codereview.chromium.org/1156323003/diff/60001/Source/devtools/front_e...
Source/devtools/front_end/elements/AnimationControlPane.js:144: if (event &&
this._animationTimeline)
On 2015/06/05 at 14:34:34, pfeldman wrote:
> When is event undefined?

Called in WebInspector.AnimationControlPane.ButtonProvider().

https://codereview.chromium.org/1156323003/diff/60001/Source/devtools/front_e...
File Source/devtools/front_end/elements/AnimationTimeline.js (right):

https://codereview.chromium.org/1156323003/diff/60001/Source/devtools/front_e...
Source/devtools/front_end/elements/AnimationTimeline.js:163:
_playbackLabelInput: function(event)
On 2015/06/05 at 14:34:34, pfeldman wrote:
> I applied your change and I can now enter random text into that field. Please
test your changes thoroughly.

I did test it thoroughly. This is intended. I can't see anywhere else we
restrict input like this. eg. metrics, you can enter random text. To commit a
value, enter/focus out must be used. Otherwise, you'll have the page jumping
around as you type a playback value. Once it has been committed, the field
updates, removing random text.

https://codereview.chromium.org/1156323003/diff/60001/Source/devtools/front_e...
Source/devtools/front_end/elements/AnimationTimeline.js:212:
this._controlButton.setTitle("Play timeline");
On 2015/06/05 at 14:34:34, pfeldman wrote:
> UIString please.

Done.

[samli, 2015-06-10 07:22:43]

ping, I would still like this in.

[pfeldman, 2015-06-10 07:49:44]

We should fix it in metrics then. What numbers do you expect input to accept?

[pfeldman, 2015-06-29 12:21:50]

pfeldman@chromium.org changed reviewers:
- pfeldman@chromium.org

[samli, 2015-07-06 08:13:54]

On 2015/06/10 at 07:49:44, pfeldman wrote:
> We should fix it in metrics then. What numbers do you expect input to accept?

In metrics? I think it is just any float number. Here: float number combined
with optional 'x'.

[samli, 2015-07-07 01:41:16]

samli@chromium.org changed reviewers:
+ pfeldman@chromium.org

[pfeldman, 2015-07-07 13:39:14]

> In metrics? I think it is just any float number. Here: float number combined
> with optional 'x'.

I don't think it makes sense to enter it manually. Can we split this change into
the button unification and editable field?

[samli, 2015-07-08 04:02:57]

On 2015/07/07 at 13:39:14, pfeldman wrote:
> > In metrics? I think it is just any float number. Here: float number combined
> > with optional 'x'.
> 
> I don't think it makes sense to enter it manually. Can we split this change
into the button unification and editable field?

Split.

It does not make sense to manually enter playback rate? It's great for
fine-grained control eg. when 0.1x is still too fast.

[samli, 2015-07-23 01:18:36]

ping

[samli, 2015-09-02 22:01:29]

ping

[pfeldman, 2015-09-03 19:05:02]

lgtm

https://codereview.chromium.org/1156323003/diff/200001/Source/devtools/front_...
File Source/devtools/front_end/animation/AnimationTimeline.js (right):

https://codereview.chromium.org/1156323003/diff/200001/Source/devtools/front_...
Source/devtools/front_end/animation/AnimationTimeline.js:173:
WebInspector.AnimationModel.fromTarget(target).setPlaybackRate(this._playbackRate());
It does not seem to be consistent with the other code I've seen where you've
been updating all targets.

[samli, 2015-09-03 22:04:18]

https://codereview.chromium.org/1156323003/diff/200001/Source/devtools/front_...
File Source/devtools/front_end/animation/AnimationTimeline.js (right):

https://codereview.chromium.org/1156323003/diff/200001/Source/devtools/front_...
Source/devtools/front_end/animation/AnimationTimeline.js:173:
WebInspector.AnimationModel.fromTarget(target).setPlaybackRate(this._playbackRate());
On 2015/09/03 at 19:05:01, pfeldman wrote:
> It does not seem to be consistent with the other code I've seen where you've
been updating all targets.

Done. Removed all instances of .mainTarget()

[samli, 2015-09-03 22:04:20]

The CQ bit was checked by samli@chromium.org

[samli, 2015-09-03 22:04:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
maxwalker@chromium.org, pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/1156323003/#ps220001
(title: " ")

[commit-bot: I haz the power, 2015-09-03 22:04:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1156323003/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1156323003/220001

[commit-bot: I haz the power, 2015-09-03 23:02:55]

Committed patchset #12 (id:220001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201749