Bug 1482357 - Part 1. Skip the space key handling of resume/pause button when the target is itself. r=daisuke
authorMantaroh Yoshinaga <mantaroh@gmail.com>
Wed, 29 Aug 2018 12:40:48 +0900
changeset 482009 0da110967e8bd3005d157a1665a9db6cb8c9e0c4
parent 482008 74318e0ebe1ecc477cc9ffb8870420721ecc42f6
child 482010 41719b855c4bbeb8cf18cedc3f9b99d06c6c0751
push id232
push userfmarier@mozilla.com
push dateWed, 05 Sep 2018 20:45:54 +0000
reviewersdaisuke
bugs1482357
milestone63.0a1
Bug 1482357 - Part 1. Skip the space key handling of resume/pause button when the target is itself. r=daisuke The PauseResumeButton component listens to the key event and clicks event. As the results of it, space key will trigger the both of event. This patch will skip the space key handling when the event target is itself. Differential Revision: https://phabricator.services.mozilla.com/D4305
devtools/client/inspector/animation/components/PauseResumeButton.js
devtools/client/inspector/animation/test/browser_animation_pause-resume-button_spacebar.js
devtools/client/inspector/animation/test/head.js
--- a/devtools/client/inspector/animation/components/PauseResumeButton.js
+++ b/devtools/client/inspector/animation/components/PauseResumeButton.js
@@ -1,18 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
-const { PureComponent } = require("devtools/client/shared/vendor/react");
+const { createRef, PureComponent } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
-const ReactDOM = require("devtools/client/shared/vendor/react-dom");
 
 const { KeyCodes } = require("devtools/client/shared/keycodes");
 
 const { getStr } = require("../utils/l10n");
 const { hasRunningAnimation } = require("../utils/utils");
 
 class PauseResumeButton extends PureComponent {
   static get propTypes() {
@@ -21,73 +20,77 @@ class PauseResumeButton extends PureComp
       setAnimationsPlayState: PropTypes.func.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
 
     this.onKeyDown = this.onKeyDown.bind(this);
+    this.pauseResumeButtonRef = createRef();
 
     this.state = {
       isRunning: false,
     };
   }
 
   componentWillMount() {
     this.updateState(this.props);
   }
 
   componentDidMount() {
     const targetEl = this.getKeyEventTarget();
     targetEl.addEventListener("keydown", this.onKeyDown);
   }
 
-  componentWillReceiveProps(nextProps) {
-    this.updateState(nextProps);
+  componentDidUpdate() {
+    this.updateState();
   }
 
   componentWillUnount() {
     const targetEl = this.getKeyEventTarget();
     targetEl.removeEventListener("keydown", this.onKeyDown);
   }
 
   getKeyEventTarget() {
-    return ReactDOM.findDOMNode(this).closest("#animation-container");
+    return this.pauseResumeButtonRef.current.closest("#animation-container");
   }
 
   onToggleAnimationsPlayState(event) {
     event.stopPropagation();
     const { setAnimationsPlayState } = this.props;
     const { isRunning } = this.state;
 
     setAnimationsPlayState(!isRunning);
   }
 
   onKeyDown(event) {
-    if (event.keyCode === KeyCodes.DOM_VK_SPACE) {
+    // Prevent to the duplicated call from the key listener and click listener.
+    if (event.keyCode === KeyCodes.DOM_VK_SPACE &&
+        event.target !== this.pauseResumeButtonRef.current) {
       this.onToggleAnimationsPlayState(event);
     }
   }
 
-  updateState(props) {
-    const { animations } = props;
+  updateState() {
+    const { animations } = this.props;
     const isRunning = hasRunningAnimation(animations);
     this.setState({ isRunning });
   }
 
   render() {
     const { isRunning } = this.state;
 
     return dom.button(
       {
         className: "pause-resume-button devtools-button" +
                    (isRunning ? "" : " paused"),
         onClick: this.onToggleAnimationsPlayState.bind(this),
         title: isRunning ?
                  getStr("timeline.resumedButtonTooltip") :
                  getStr("timeline.pausedButtonTooltip"),
+        ref: this.pauseResumeButtonRef,
       }
     );
   }
 }
 
 module.exports = PauseResumeButton;
--- a/devtools/client/inspector/animation/test/browser_animation_pause-resume-button_spacebar.js
+++ b/devtools/client/inspector/animation/test/browser_animation_pause-resume-button_spacebar.js
@@ -7,27 +7,35 @@
 // * make animations to pause/resume by spacebar
 // * combination with other UI components
 
 add_task(async function() {
   await addTab(URL_ROOT + "doc_custom_playback_rate.html");
   const { animationInspector, panel } = await openAnimationInspector();
 
   info("Checking spacebar makes animations to pause");
-  await sendSpaceKeyEvent(animationInspector, panel);
-  assertAnimationsPausing(animationInspector, panel);
-  await sendSpaceKeyEvent(animationInspector, panel);
-  assertAnimationsRunning(animationInspector, panel);
+  await testPauseAndResumeBySpacebar(animationInspector, panel);
+
+  info("Checking spacebar makes animations to pause when the button has the focus");
+  const pauseResumeButton = panel.querySelector(".pause-resume-button");
+  await testPauseAndResumeBySpacebar(animationInspector, pauseResumeButton);
 
   info("Checking spacebar works with other UI components");
   // To pause
   await clickOnPauseResumeButton(animationInspector, panel);
   // To resume
   await sendSpaceKeyEvent(animationInspector, panel);
   assertAnimationsRunning(animationInspector, panel);
   // To pause
   await clickOnCurrentTimeScrubberController(animationInspector, panel, 0.5);
   // To resume
   await clickOnPauseResumeButton(animationInspector, panel);
   // To pause
   await sendSpaceKeyEvent(animationInspector, panel);
   assertAnimationsPausing(animationInspector, panel);
 });
+
+async function testPauseAndResumeBySpacebar(animationInspector, element) {
+  await sendSpaceKeyEvent(animationInspector, element);
+  assertAnimationsPausing(animationInspector, element);
+  await sendSpaceKeyEvent(animationInspector, element);
+  assertAnimationsRunning(animationInspector, element);
+}
--- a/devtools/client/inspector/animation/test/head.js
+++ b/devtools/client/inspector/animation/test/head.js
@@ -436,22 +436,21 @@ const selectNodeAndWaitForAnimations = a
   await onUpdated;
   await waitForRendering(inspector.animationinspector);
 };
 
 /**
  * Send keyboard event of space to given panel.
  *
  * @param {AnimationInspector} animationInspector
- * @param {DOMElement} panel
- *        #animation-container element.
+ * @param {DOMElement} target element.
  */
-const sendSpaceKeyEvent = async function(animationInspector, panel) {
-  panel.focus();
-  EventUtils.sendKey("SPACE", panel.ownerGlobal);
+const sendSpaceKeyEvent = async function(animationInspector, element) {
+  element.focus();
+  EventUtils.sendKey("SPACE", element.ownerGlobal);
   await waitForSummaryAndDetail(animationInspector);
 };
 
 /**
  * Set a node class attribute to the given selector.
  *
  * @param {AnimationInspector} animationInspector
  * @param {String} selector