Bug 1210796 - Part 15: Displays the animation detail if the previously displayed animation is included in timeline list after refresh all UI. r=pbro draft
authorDaisuke Akatsuka <dakatsuka@mozilla.com>
Tue, 18 Apr 2017 12:15:57 +0900
changeset 564128 38e2e3d2ac8330246354f6e33cd4d3e6cffd7100
parent 564127 24990eab85d69b26128d05a2523b5241b82d69ba
child 564129 4ba59333bbe59b6a55d1ce53da20e5660fd6ca8c
push id54524
push userbmo:dakatsuka@mozilla.com
push dateTue, 18 Apr 2017 09:24:06 +0000
reviewerspbro
bugs1210796
milestone55.0a1
Bug 1210796 - Part 15: Displays the animation detail if the previously displayed animation is included in timeline list after refresh all UI. r=pbro MozReview-Commit-ID: FiltMpwd4VY
devtools/client/animationinspector/components/animation-timeline.js
devtools/client/animationinspector/test/browser_animation_animated_properties_for_delayed_starttime_animations.js
devtools/client/animationinspector/test/browser_animation_detail_displayed.js
devtools/client/animationinspector/test/browser_animation_timeline_pause_button_03.js
devtools/client/animationinspector/test/head.js
--- a/devtools/client/animationinspector/components/animation-timeline.js
+++ b/devtools/client/animationinspector/components/animation-timeline.js
@@ -253,16 +253,17 @@ AnimationsTimeline.prototype = {
     this.win = null;
     this.inspector = null;
     this.serverTraits = null;
     this.animationDetailEl = null;
     this.animationAnimationNameEl = null;
     this.animatedPropertiesEl = null;
     this.animationDetailCloseButton = null;
     this.animationRootEl = null;
+    this.selectedAnimation = null;
   },
 
   /**
    * Destroy sub-components that have been created and stored on this instance.
    * @param {String} name An array of components will be expected in this[name]
    * @param {Array} handlers An option list of event handlers information that
    * should be used to remove these handlers.
    */
@@ -272,24 +273,28 @@ AnimationsTimeline.prototype = {
         component.off(event, fn);
       }
       component.destroy();
     }
     this[name] = [];
   },
 
   unrender: function () {
+    this.unrenderButLeaveDetailsComponent();
+    this.details.unrender();
+  },
+
+  unrenderButLeaveDetailsComponent: function () {
     for (let animation of this.animations) {
       animation.off("changed", this.onAnimationStateChanged);
     }
     this.stopAnimatingScrubber();
     TimeScale.reset();
     this.destroySubComponents("targetNodes");
     this.destroySubComponents("timeBlocks");
-    this.details.unrender();
     this.animationsEl.innerHTML = "";
     this.off("timeline-data-changed", this.onTimelineDataChanged);
   },
 
   onWindowResize: function () {
     // Don't do anything if the root element has a width of 0
     if (this.rootWrapperEl.offsetWidth === 0) {
       return;
@@ -332,19 +337,23 @@ AnimationsTimeline.prototype = {
       this.animatedPropertiesEl.className =
         `animated-properties ${ animation.state.type }`;
     }
 
     // Select and render.
     const selectedAnimationEl = animationEls[index];
     selectedAnimationEl.classList.add("selected");
     this.animationRootEl.classList.add("animation-detail-visible");
-    yield this.details.render(animation);
+    if (animation !== this.details.animation) {
+      this.selectedAnimation = animation;
+      // Don't render if the detail displays same animation already.
+      yield this.details.render(animation);
+      this.animationAnimationNameEl.textContent = getFormattedAnimationTitle(animation);
+    }
     this.onTimelineDataChanged(null, { time: this.currentTime || 0 });
-    this.animationAnimationNameEl.textContent = getFormattedAnimationTitle(animation);
     this.emit("animation-selected", animation);
   }),
 
   /**
    * When move the scrubber to the corresponding position
    */
   onScrubberMouseDown: function (e) {
     this.moveScrubberTo(e.pageX);
@@ -416,17 +425,17 @@ AnimationsTimeline.prototype = {
         ? " some-properties"
         : " all-properties";
     }
 
     return className;
   },
 
   render: function (animations, documentCurrentTime) {
-    this.unrender();
+    this.unrenderButLeaveDetailsComponent();
 
     this.animations = animations;
     if (!this.animations.length) {
       return;
     }
 
     // Loop first to set the time scale for all current animations.
     for (let {state} of animations) {
@@ -491,20 +500,31 @@ AnimationsTimeline.prototype = {
       this.startAnimatingScrubber(this.wasRewound()
                                   ? TimeScale.minStartTime
                                   : documentCurrentTime);
     }
 
     // To indicate the animation progress in AnimationDetails.
     this.on("timeline-data-changed", this.onTimelineDataChanged);
 
-    // Display animation's detail if there is only one animation.
+    // Display animation's detail if there is only one animation,
+    // or the previously displayed animation is included in timeline list.
     if (this.animations.length === 1) {
       this.onAnimationSelected(null, this.animations[0]);
+      return;
     }
+    if (!this.animationRootEl.classList.contains("animation-detail-visible")) {
+      // Do nothing since the animation detail pane is closing now.
+      return;
+    }
+    if (this.animations.indexOf(this.selectedAnimation) >= 0) {
+      this.onAnimationSelected(null, this.selectedAnimation);
+      return;
+    }
+    this.onDetailCloseButtonClick();
   },
 
   isAtLeastOneAnimationPlaying: function () {
     return this.animations.some(({state}) => state.playState === "running");
   },
 
   wasRewound: function () {
     return !this.isAtLeastOneAnimationPlaying() &&
--- a/devtools/client/animationinspector/test/browser_animation_animated_properties_for_delayed_starttime_animations.js
+++ b/devtools/client/animationinspector/test/browser_animation_animated_properties_for_delayed_starttime_animations.js
@@ -1,28 +1,34 @@
 /* vim: set ts=2 et sw=2 tw=80: */
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 // Test for animations that have different starting time.
 // We should check progress indicator working well even if the start time is not zero.
+// Also, check that there is no duplication display.
 
 add_task(function* () {
   yield addTab(URL_ROOT + "doc_delayed_starttime_animations.html");
   const { panel } = yield openAnimationInspector();
-  yield setStyle(null, panel, "animation", "anim 100s", "#target2");
-  yield setStyle(null, panel, "animation", "anim 100s", "#target3");
-  yield setStyle(null, panel, "animation", "anim 100s", "#target4");
-  yield setStyle(null, panel, "animation", "anim 100s", "#target5");
+  yield setStyleAndWaitForAnimationSelecting(panel, "animation", "anim 100s", "#target2");
+  yield setStyleAndWaitForAnimationSelecting(panel, "animation", "anim 100s", "#target3");
+  yield setStyleAndWaitForAnimationSelecting(panel, "animation", "anim 100s", "#target4");
+  yield setStyleAndWaitForAnimationSelecting(panel, "animation", "anim 100s", "#target5");
 
-  yield clickOnAnimation(panel, 1);
   const timelineComponent = panel.animationsTimelineComponent;
   const detailsComponent = timelineComponent.details;
+  const headers =
+    detailsComponent.containerEl.querySelectorAll(".animated-properties-header");
+  is(headers.length, 1, "There should be only one header in the details panel");
+
+  // Check indicator.
+  yield clickOnAnimation(panel, 1);
   const progressIndicatorEl = detailsComponent.progressIndicatorEl;
   const startTime = detailsComponent.animation.state.previousStartTime;
   detailsComponent.indicateProgress(0);
   is(progressIndicatorEl.style.left, "0%",
      "The progress indicator position should be 0% at 0ms");
   detailsComponent.indicateProgress(startTime);
   is(progressIndicatorEl.style.left, "0%",
      "The progress indicator position should be 0% at start time");
@@ -31,8 +37,14 @@ add_task(function* () {
      "The progress indicator position should be 50% at half time of animation");
   detailsComponent.indicateProgress(startTime + 99 * 1000);
   is(progressIndicatorEl.style.left, "99%",
      "The progress indicator position should be 99% at 99s");
   detailsComponent.indicateProgress(startTime + 100 * 1000);
   is(progressIndicatorEl.style.left, "0%",
      "The progress indicator position should be 0% at end of animation");
 });
+
+function* setStyleAndWaitForAnimationSelecting(panel, name, value, selector) {
+  const onSelecting = waitForAnimationSelecting(panel);
+  yield setStyle(null, panel, name, value, selector);
+  yield onSelecting;
+}
--- a/devtools/client/animationinspector/test/browser_animation_detail_displayed.js
+++ b/devtools/client/animationinspector/test/browser_animation_detail_displayed.js
@@ -6,16 +6,17 @@
 
 // Tests the behavior of animation-detail container.
 // We test following cases.
 // 1. Existance of animation-detail element.
 // 2. Hidden at first if multiple animations were displayed.
 // 3. Display after click on an animation.
 // 4. Display from first time if displayed animation is only one.
 // 5. Close the animation-detail element by clicking on close button.
+// 6. Stay selected animation even if refresh all UI.
 
 requestLongerTimeout(5);
 
 add_task(function* () {
   yield addTab(URL_ROOT + "doc_multiple_property_types.html");
   const { panel, inspector } = yield openAnimationInspector();
   const timelineComponent = panel.animationsTimelineComponent;
   const animationDetailEl =
@@ -52,9 +53,15 @@ add_task(function* () {
      "animation-detail element should not display");
 
   // Select another animation.
   yield selectNodeAndWaitForAnimations("#target2", inspector);
   isnot(win.getComputedStyle(splitboxControlledEl).display, "none",
         "animation-detail element should display");
   is(animationDetailEl.offsetHeight, previousHeight,
      "The height of animation-detail should keep the height");
+
+  // 6. Stay selected animation even if refresh all UI.
+  yield selectNodeAndWaitForAnimations("#target1", inspector);
+  yield clickTimelineRewindButton(panel);
+  ok(animationDetailEl.querySelector(".property"),
+     "The property in animation-detail element should stay as is");
 });
--- a/devtools/client/animationinspector/test/browser_animation_timeline_pause_button_03.js
+++ b/devtools/client/animationinspector/test/browser_animation_timeline_pause_button_03.js
@@ -35,18 +35,16 @@ add_task(function* () {
   yield assertScrubberMoving(panel, false);
 
   info("Click again on the button to play the animation from the start again");
   yield clickTimelinePlayPauseButton(panel);
 
   ok(!btn.classList.contains("paused"),
      "Clicking the button once finite animations are done should restart them");
   yield assertScrubberMoving(panel, true);
-
-  yield waitForAnimationSelecting(panel);
 });
 
 function waitForButtonPaused(btn) {
   return new Promise(resolve => {
     let observer = new btn.ownerDocument.defaultView.MutationObserver(mutations => {
       for (let mutation of mutations) {
         if (mutation.type === "attributes" &&
             mutation.attributeName === "class" &&
--- a/devtools/client/animationinspector/test/head.js
+++ b/devtools/client/animationinspector/test/head.js
@@ -470,15 +470,13 @@ function* setStyle(animation, panel, nam
 
   const onAnimationChanged = animation ? once(animation, "changed") : Promise.resolve();
   yield executeInContent("devtools:test:setStyle", {
     selector: selector,
     propertyName: name,
     propertyValue: value
   });
   yield onAnimationChanged;
-  const onSelected = animation ? waitForAnimationSelecting(panel) : Promise.resolve();
-  yield onSelected;
 
   // Also wait for the target node previews to be loaded if the panel got
   // refreshed as a result of this animation mutation.
   yield waitForAllAnimationTargets(panel);
 }