Bug 1210796 - Part 7: Change selection logic. r=pbro draft
authorDaisuke Akatsuka <daisuke@mozilla-japan.org>
Tue, 18 Apr 2017 12:15:55 +0900
changeset 564120 506b53318e6668f9cc8bcda8cae2a4923e193efc
parent 564119 f7d9f699f73aadbd48f9f1726dc397386b172c66
child 564121 767ce7f132b549bb2bc5d0b0b3f60d29c36f0bf9
push id54524
push userbmo:dakatsuka@mozilla.com
push dateTue, 18 Apr 2017 09:24:06 +0000
reviewerspbro
bugs1210796
milestone55.0a1
Bug 1210796 - Part 7: Change selection logic. r=pbro MozReview-Commit-ID: BgG2Hjf0gP4
devtools/client/animationinspector/components/animation-timeline.js
devtools/client/animationinspector/test/browser_animation_animated_properties_displayed.js
devtools/client/animationinspector/test/browser_animation_click_selects_animation.js
devtools/client/animationinspector/test/head.js
--- a/devtools/client/animationinspector/components/animation-timeline.js
+++ b/devtools/client/animationinspector/components/animation-timeline.js
@@ -291,40 +291,47 @@ AnimationsTimeline.prototype = {
   },
 
   onAnimationSelected: function (e, animation) {
     let index = this.animations.indexOf(animation);
     if (index === -1) {
       return;
     }
 
-    let el = this.rootWrapperEl;
-    let animationEl = el.querySelectorAll(".animation")[index];
-
-    // Toggle the selected state on this animation.
-    animationEl.classList.toggle("selected");
+    // Unselect an animation which was selected.
+    const animationEls = this.rootWrapperEl.querySelectorAll(".animation");
+    for (let i = 0; i < animationEls.length; i++) {
+      const animationEl = animationEls[i];
+      if (!animationEl.classList.contains("selected")) {
+        continue;
+      }
+      if (i === index) {
+        // Already the animation is selected.
+        this.emit("animation-already-selected", this.animations[i]);
+        return;
+      }
+      animationEl.classList.remove("selected");
+      this.emit("animation-unselected", this.animations[i]);
+      break;
+    }
 
-    // Render the details component for this animation if it was shown.
-    if (animationEl.classList.contains("selected")) {
-      // Add class of animation type.
-      if (!this.animatedPropertiesEl.classList.contains(animation.state.type)) {
-        this.animatedPropertiesEl.className =
-          `animated-properties ${ animation.state.type }`;
-      }
-      this.animationDetailEl.style.display =
-        this.animationDetailEl.dataset.defaultDisplayStyle;
-      this.details.render(animation);
-      this.emit("animation-selected", animation);
+    // Add class of animation type to animatedPropertiesEl to display the compositor sign.
+    if (!this.animatedPropertiesEl.classList.contains(animation.state.type)) {
+      this.animatedPropertiesEl.className =
+        `animated-properties ${ animation.state.type }`;
+    }
 
-      this.animationAnimationNameEl.textContent =
-        getFormattedAnimationTitle(animation);
-    } else {
-      this.emit("animation-unselected", animation);
-      this.animationDetailEl.style.display = "none";
-    }
+    // Select and render.
+    const selectedAnimationEl = animationEls[index];
+    selectedAnimationEl.classList.add("selected");
+    this.animationDetailEl.style.display =
+      this.animationDetailEl.dataset.defaultDisplayStyle;
+    this.details.render(animation);
+    this.animationAnimationNameEl.textContent = getFormattedAnimationTitle(animation);
+    this.emit("animation-selected", animation);
   },
 
   /**
    * When a frame gets selected, move the scrubber to the corresponding position
    */
   onFrameSelected: function (e, {x}) {
     this.moveScrubberTo(x, true);
   },
--- a/devtools/client/animationinspector/test/browser_animation_animated_properties_displayed.js
+++ b/devtools/client/animationinspector/test/browser_animation_animated_properties_displayed.js
@@ -47,16 +47,21 @@ add_task(function* () {
      "The list of properties panel is shown");
   ok(propertiesList.querySelectorAll(".property").length,
      "The list of properties panel actually contains properties");
   ok(hasExpectedProperties(propertiesList),
      "The list of properties panel contains the right properties");
 
   ok(hasExpectedWarnings(propertiesList),
      "The list of properties panel contains the right warnings");
+
+  info("Click same animation again");
+  yield clickOnAnimation(panel, 0, true);
+  ok(isNodeVisible(propertiesList),
+     "The list of properties panel keeps");
 });
 
 function hasExpectedProperties(containerEl) {
   let names = [...containerEl.querySelectorAll(".property .name")]
               .map(n => n.textContent)
               .sort();
 
   if (names.length !== EXPECTED_PROPERTIES.length) {
--- a/devtools/client/animationinspector/test/browser_animation_click_selects_animation.js
+++ b/devtools/client/animationinspector/test/browser_animation_click_selects_animation.js
@@ -23,19 +23,23 @@ add_task(function* () {
      "The time block has the right selected class");
 
   info("Click on the second animation, expect it to be selected too");
   let animation1 = yield clickOnAnimation(panel, 1);
   is(animation1, timeline.animations[1],
      "The selected event was emitted with the right animation");
   ok(isTimeBlockSelected(timeline, 1),
      "The second time block has the right selected class");
+  ok(!isTimeBlockSelected(timeline, 0),
+     "The first time block has been unselected");
 
   info("Click again on the first animation and check if it unselects");
-  yield clickOnAnimation(panel, 0, true);
-  ok(!isTimeBlockSelected(timeline, 0),
-     "The first time block has been unselected");
+  yield clickOnAnimation(panel, 0);
+  ok(isTimeBlockSelected(timeline, 0),
+     "The time block has the right selected class again");
+  ok(!isTimeBlockSelected(timeline, 1),
+     "The second time block has been unselected");
 });
 
 function isTimeBlockSelected(timeline, index) {
   let animation = timeline.rootWrapperEl.querySelectorAll(".animation")[index];
   return animation.classList.contains("selected");
 }
--- a/devtools/client/animationinspector/test/head.js
+++ b/devtools/client/animationinspector/test/head.js
@@ -364,31 +364,31 @@ function disableHighlighter(toolbox) {
     traits: {}
   };
 }
 
 /**
  * Click on an animation in the timeline to select/unselect it.
  * @param {AnimationsPanel} panel The panel instance.
  * @param {Number} index The index of the animation to click on.
- * @param {Boolean} shouldClose Set to true if clicking should close the
- * animation.
+ * @param {Boolean} shouldAlreadySelected Set to true
+ *                  if the clicked animation is already selected.
  * @return {Promise} resolves to the animation whose state has changed.
  */
-function* clickOnAnimation(panel, index, shouldClose) {
+function* clickOnAnimation(panel, index, shouldAlreadySelected) {
   let timeline = panel.animationsTimelineComponent;
 
   // Expect a selection event.
-  let onSelectionChanged = timeline.once(shouldClose
-                                         ? "animation-unselected"
+  let onSelectionChanged = timeline.once(shouldAlreadySelected
+                                         ? "animation-already-selected"
                                          : "animation-selected");
 
   // If we're opening the animation, also wait for
   // the animation-detail-rendering-completed event.
-  let onReady = shouldClose
+  let onReady = shouldAlreadySelected
                 ? Promise.resolve()
                 : timeline.details.once("animation-detail-rendering-completed");
 
   info("Click on animation " + index + " in the timeline");
   let timeBlock = timeline.rootWrapperEl.querySelectorAll(".time-block")[index];
   EventUtils.sendMouseEvent({type: "click"}, timeBlock,
                             timeBlock.ownerDocument.defaultView);