Devtools Animations: Basic animation inspection & control in Styles pane

Source/devtools/front_end/elements/AnimationSection.js

Index: Source/devtools/front_end/elements/AnimationSection.js
diff --git a/Source/devtools/front_end/elements/AnimationSection.js b/Source/devtools/front_end/elements/AnimationSection.js
new file mode 100644
index 0000000000000000000000000000000000000000..a51b0c946b710823c15fe2dc19ea24bfa444be64
--- /dev/null
+++ b/Source/devtools/front_end/elements/AnimationSection.js
@@ -0,0 +1,128 @@
+/**

[apavlov, 2014/10/01 07:39:36]

The [short 3-line] copyright notice is missing

[samli, 2014/10/02 08:30:33]

Done.

+ * @constructor
+ * @param {?WebInspector.DOMModel.AnimationPlayer} animationPlayer
+ */
+WebInspector.AnimationSection = function(animationPlayer)
+{
+    var section = this;

[apavlov, 2014/10/01 07:39:35]

This should go just before the createCurrentTimeSlider(), since it's only used
in nested functions

[samli, 2014/10/02 08:30:33]

Done.

+    this.element = document.createElement("div");
+    this.element.className = "styles-section";
+    this.element._section = this;
+    this.propertiesElement = document.createElement("div");
+    this.player = animationPlayer;

[apavlov, 2014/10/01 07:39:36]

We typically assign the constructor arguments first

[samli, 2014/10/02 08:30:33]

Done.

+    this.animationControls = createAnimationControls();
+    this.element.appendChild(this.animationControls);
+    this.element.appendChild(this.propertiesElement);
+
+    /**
+     * @return {!Element}
+     */
+    function createCurrentTimeSlider() {

[apavlov, 2014/10/01 07:39:35]

Brace on the next line. I also think this should be a private method, not a
nested function. This will allow the use of "this", and we'll get rid of "var
section"

[samli, 2014/10/02 08:30:34]

Done.

+        var slider = document.createElement("input");
+        var iterationDuration = section.player.animation().duration();
+        var iterationCount = section.player.animation().iterationCount();
+        slider.type = "range";

[apavlov, 2014/10/01 07:39:35]

It's best to glue these to the "var slider" declaration

[samli, 2014/10/02 08:30:33]

Done.

+        slider.min = 0;
+        slider.step = 0.01;
+
+        if (!iterationCount) {
+            // Infinite iterations
+            slider.max = iterationDuration;
+            slider.value = section.player.currentTime() % iterationDuration;
+        } else {
+            slider.max = iterationCount * iterationDuration;
+            slider.value = section.player.currentTime();
+        }
+
+        slider.addEventListener('input', function (e) {

[apavlov, 2014/10/01 07:39:35]

DevTools use double quotes for string literals.

This should also be a named function (since it will allow the proper annotation
of |e|)

[samli, 2014/10/02 08:30:33]

Done.

+            section.player.setCurrentTime(parseFloat(e.target.value), updateAnimationPlayer);
+        });
+        requestAnimationFrame(updateCurrentTime);
+        return slider;
+    }
+
+    /**
+     * @return {!Element}
+     */
+    function createAnimationControls() {

[apavlov, 2014/10/01 07:39:35]

Brace on the next line. Ditto for this becoming a private method.

[samli, 2014/10/02 08:30:33]

Done.

+        var controls = document.createElement("div");
+
+        // Pause/play button
+        var pausePlayButton = document.createElement("button");

[apavlov, 2014/10/01 07:39:35]

Not sure, but you might want to take a look at WebInspector.StatusBarButton
instead

[samli, 2014/10/02 08:30:33]

Ack. Going to look into this tomorrow.

+        if (section.player.paused()) {

[apavlov, 2014/10/01 07:39:36]

no braces around single-line blocks

[samli, 2014/10/02 08:30:33]

Done.

+            pausePlayButton.textContent = WebInspector.UIString("Play");
+        } else {

[apavlov, 2014/10/01 07:39:36]

ditto

[samli, 2014/10/02 08:30:34]

Done.

+            pausePlayButton.textContent = WebInspector.UIString("Pause");
+        }
+        pausePlayButton.addEventListener("click", function () {

[apavlov, 2014/10/01 07:39:35]

Please use a named nested function

[samli, 2014/10/02 08:30:33]

Done.

+            if (section.player.paused()) {
+                section.player.play(updateAnimationPlayer);
+                pausePlayButton.textContent = WebInspector.UIString("Pause");
+            } else {
+                section.player.pause(updateAnimationPlayer);
+                pausePlayButton.textContent = WebInspector.UIString("Play");
+            }
+        });
+        controls.appendChild(pausePlayButton);
+
+        // Current time slider
+        section.currentTimeSlider = createCurrentTimeSlider();
+        controls.appendChild(section.currentTimeSlider);
+
+        return controls;
+    }
+
+    /**
+     * @param {?WebInspector.DOMModel.AnimationPlayer} animationPlayer
+     */
+    function updateAnimationPlayer(animationPlayer) {

[apavlov, 2014/10/01 07:39:35]

brace on the next line

[samli, 2014/10/02 08:30:33]

Done.

+        if (animationPlayer)

[apavlov, 2014/10/01 07:39:36]

Is this missing braces around the next two lines or is the
"requestAnimationFrame" call improperly indented?

[samli, 2014/10/02 08:30:33]

Acknowledged.

+            section.player = animationPlayer;
+            requestAnimationFrame(updateCurrentTime);
+    }
+
+    function updateCurrentTime() {

[apavlov, 2014/10/01 07:39:35]

brace on the next line

[samli, 2014/10/02 08:30:33]

Done.

+        section.player.getCurrentState(function (currentTime, isRunning) {

[apavlov, 2014/10/01 07:39:36]

Annotated named function, please

[samli, 2014/10/02 08:30:33]

Done.

+            section.currentTimeSlider.value = section.player.animation().iterationCount() == null ? currentTime % section.player.animation().duration() : currentTime;
+            if (isRunning) {

[apavlov, 2014/10/01 07:39:35]

no braces around single-line blocks

[samli, 2014/10/02 08:30:34]

Done.

+                requestAnimationFrame(updateCurrentTime);
+            }
+        });
+    }
+}
+
+WebInspector.AnimationSection.prototype = {
+    get player()

[apavlov, 2014/10/01 07:39:36]

We do not use getters and setters in the new code. Please turn these into
functions

[samli, 2014/10/02 08:30:33]

Done.

+    {
+        return this._animationPlayer;
+    },
+    set player(p)

[apavlov, 2014/10/01 07:39:35]

blank line above

[samli, 2014/10/02 08:30:33]

Done.

+    {
+        if (p === this._animationPlayer)
+            return;
+
+        this._animationPlayer = p;
+
+        if (p instanceof WebInspector.DOMModel.AnimationPlayer) {
+            this.propertiesElement.removeChildren();
+            var animationObject = {
+                'paused': p.paused(),

[apavlov, 2014/10/01 07:39:36]

Double-quotes for string literals

[samli, 2014/10/02 08:30:33]

Done.

+                'finished': p.finished(),
+                'start-time': p.startTime(),
+                'player-playback-rate': p.playbackRate(),
+                'id': p.sequenceNumber(),
+                'start-delay': p.animation().startDelay(),
+                'playback-rate': p.animation().playbackRate(),
+                'iteration-start': p.animation().iterationStart(),
+                'iteration-count': p.animation().iterationCount(),
+                'duration': p.animation().duration(),
+                'direction': p.animation().direction(),
+                'fill-mode': p.animation().fillMode(),
+                'time-fraction': p.animation().timeFraction()
+            };
+            var obj = WebInspector.RemoteObject.fromLocalObject(animationObject);
+            var section = new WebInspector.ObjectPropertiesSection(obj, WebInspector.UIString("Animation Properties"), undefined, null, undefined, undefined);

[apavlov, 2014/10/01 07:39:36]

You only need the first two arguments here. Please remove the rest.

[samli, 2014/10/02 08:30:33]

Done.

+            this.propertiesElement.appendChild(section.element);
+        }
+    },

[apavlov, 2014/10/01 07:39:35]

extra comma

[samli, 2014/10/02 08:30:33]

Done.

+}