Bug 1300473 - Properly set recording buttons classes in performance tool. r=gregtatum
authorTim Nguyen <ntim.bugs@gmail.com>
Mon, 12 Sep 2016 17:31:51 +0200
changeset 313507 68be9a5221854bcb29884f648bcebc706125e943
parent 313506 b8cba97b26485efbeab6530cefa75bc0a3365c0f
child 313508 7826ad7df05fb73b79f808885ed4ddea9210da55
child 313541 b1156b0eb96fcb49966b20e5fcf6a01f634ea2ee
push id20515
push userntim.bugs@gmail.com
push dateMon, 12 Sep 2016 19:18:12 +0000
treeherderfx-team@68be9a522185 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgregtatum
bugs1300473
milestone51.0a1
Bug 1300473 - Properly set recording buttons classes in performance tool. r=gregtatum
devtools/client/performance/components/recording-button.js
devtools/client/performance/components/recording-controls.js
devtools/client/performance/test/browser_perf-button-states.js
devtools/client/performance/test/browser_perf-recording-selected-03.js
devtools/client/themes/common.css
devtools/client/themes/performance.css
--- a/devtools/client/performance/components/recording-button.js
+++ b/devtools/client/performance/components/recording-button.js
@@ -8,22 +8,30 @@ const {DOM, createClass} = require("devt
 const {button} = DOM;
 
 module.exports = createClass({
   displayName: "Recording Button",
 
   render() {
     let {
       onRecordButtonClick,
-      isRecording
+      isRecording,
+      isLocked
     } = this.props;
 
+    let classList = ["devtools-button", "record-button"];
+
+    if (isRecording) {
+      classList.push("checked");
+    }
+
     return button(
       {
-        className: "devtools-toolbarbutton record-button",
+        className: classList.join(" "),
         onClick: onRecordButtonClick,
         "data-standalone": "true",
         "data-text-only": "true",
+        disabled: isLocked
       },
       isRecording ? L10N.getStr("recordings.stop") : L10N.getStr("recordings.start")
     );
   }
 });
--- a/devtools/client/performance/components/recording-controls.js
+++ b/devtools/client/performance/components/recording-controls.js
@@ -5,62 +5,46 @@
 
 const {L10N} = require("devtools/client/performance/modules/global");
 const {DOM, createClass} = require("devtools/client/shared/vendor/react");
 const {div, button} = DOM;
 
 module.exports = createClass({
   displayName: "Recording Controls",
 
-  /**
-   * Manually handle the "checked" and "locked" attributes, as the DOM element won't
-   * change by just by changing the checked attribute through React.
-   */
-  componentDidUpdate() {
-    if (this.props.isRecording) {
-      this._recordButton.setAttribute("checked", true);
-    } else {
-      this._recordButton.removeAttribute("checked");
-    }
-
-    if (this.props.isLocked) {
-      this._recordButton.setAttribute("locked", true);
-    } else {
-      this._recordButton.removeAttribute("locked");
-    }
-  },
-
   render() {
     let {
       onClearButtonClick,
       onRecordButtonClick,
       onImportButtonClick,
       isRecording,
       isLocked
     } = this.props;
 
+    let recordButtonClassList = ["devtools-button", "record-button"];
+
+    if (isRecording) {
+      recordButtonClassList.push("checked");
+    }
+
     return (
       div({ className: "devtools-toolbar" },
         div({ className: "toolbar-group" },
           button({
             id: "clear-button",
             className: "devtools-button",
             title: L10N.getStr("recordings.clear.tooltip"),
             onClick: onClearButtonClick
           }),
           button({
             id: "main-record-button",
-            className: "devtools-button record-button",
+            className: recordButtonClassList.join(" "),
+            disabled: isLocked,
             title: L10N.getStr("recordings.start.tooltip"),
-            onClick: onRecordButtonClick,
-            checked: isRecording,
-            ref: (el) => {
-              this._recordButton = el;
-            },
-            locked: isLocked
+            onClick: onRecordButtonClick
           }),
           button({
             id: "import-button",
             className: "devtools-button",
             title: L10N.getStr("recordings.import.tooltip"),
             onClick: onImportButtonClick
           })
         )
--- a/devtools/client/performance/test/browser_perf-button-states.js
+++ b/devtools/client/performance/test/browser_perf-button-states.js
@@ -11,48 +11,40 @@ const { initPerformanceInNewTab, teardow
 const { once } = require("devtools/client/performance/test/helpers/event-utils");
 
 add_task(function* () {
   let { panel } = yield initPerformanceInNewTab({
     url: SIMPLE_URL,
     win: window
   });
 
-  let { $, EVENTS, PerformanceController, PerformanceView } = panel.panelWin;
+  let { $, $$, EVENTS, PerformanceController, PerformanceView } = panel.panelWin;
+
   let recordButton = $("#main-record-button");
 
-  ok(!recordButton.hasAttribute("checked"),
-    "The record button should not be checked yet.");
-  ok(!recordButton.hasAttribute("locked"),
-    "The record button should not be locked yet.");
+  checkRecordButtonsStates(false, false);
 
   let uiStartClick = once(PerformanceView, EVENTS.UI_START_RECORDING);
   let recordingStarted = once(PerformanceController, EVENTS.RECORDING_STATE_CHANGE, {
     expectedArgs: { "1": "recording-started" }
   });
   let backendStartReady = once(PerformanceController,
                                EVENTS.BACKEND_READY_AFTER_RECORDING_START);
   let uiStateRecording = once(PerformanceView, EVENTS.UI_STATE_CHANGED, {
     expectedArgs: { "1": "recording" }
   });
 
   click(recordButton);
   yield uiStartClick;
 
-  ok(recordButton.hasAttribute("checked"),
-    "The record button should now be checked.");
-  ok(recordButton.hasAttribute("locked"),
-    "The record button should be locked.");
+  checkRecordButtonsStates(true, true);
 
   yield recordingStarted;
 
-  ok(recordButton.hasAttribute("checked"),
-    "The record button should still be checked.");
-  ok(!recordButton.hasAttribute("locked"),
-    "The record button should not be locked.");
+  checkRecordButtonsStates(true, false);
 
   yield backendStartReady;
   yield uiStateRecording;
 
   let uiStopClick = once(PerformanceView, EVENTS.UI_STOP_RECORDING);
   let recordingStopped = once(PerformanceController, EVENTS.RECORDING_STATE_CHANGE, {
     expectedArgs: { "1": "recording-stopped" }
   });
@@ -61,18 +53,24 @@ add_task(function* () {
   let uiStateRecorded = once(PerformanceView, EVENTS.UI_STATE_CHANGED, {
     expectedArgs: { "1": "recorded" }
   });
 
   click(recordButton);
   yield uiStopClick;
   yield recordingStopped;
 
-  ok(!recordButton.hasAttribute("checked"),
-    "The record button should not be checked.");
-  ok(!recordButton.hasAttribute("locked"),
-    "The record button should not be locked.");
+  checkRecordButtonsStates(false, false);
 
   yield backendStopReady;
   yield uiStateRecorded;
 
   yield teardownToolboxAndRemoveTab(panel);
+
+  function checkRecordButtonsStates(checked, locked) {
+    for (let button of $$(".record-button")) {
+      is(button.classList.contains("checked"), checked,
+         "The record button checked state should be " + checked);
+      is(button.disabled, locked,
+         "The record button locked state should be " + locked);
+    }
+  }
 });
--- a/devtools/client/performance/test/browser_perf-recording-selected-03.js
+++ b/devtools/client/performance/test/browser_perf-recording-selected-03.js
@@ -27,17 +27,17 @@ add_task(function* () {
   yield startRecording(panel);
 
   info("Selecting recording #0 and waiting for it to be displayed.");
 
   let selected = once(PerformanceController, EVENTS.RECORDING_SELECTED);
   RecordingsView.selectedIndex = 0;
   yield selected;
 
-  ok($("#main-record-button").hasAttribute("checked"),
+  ok($("#main-record-button").classList.contains("checked"),
     "Button is still checked after selecting another item.");
-  ok(!$("#main-record-button").hasAttribute("locked"),
+  ok(!$("#main-record-button").hasAttribute("disabled"),
     "Button is not locked after selecting another item.");
 
   yield stopRecording(panel);
 
   yield teardownToolboxAndRemoveTab(panel);
 });
--- a/devtools/client/themes/common.css
+++ b/devtools/client/themes/common.css
@@ -362,27 +362,29 @@ checkbox:-moz-focusring {
 
 /* Icon-only buttons */
 .devtools-button:empty::before,
 .devtools-toolbarbutton:not([label]):not([disabled]) > image {
   opacity: 0.8;
 }
 
 .devtools-button:hover:empty::before,
+.devtools-button.checked:empty::before,
 .devtools-button[checked]:empty::before,
 .devtools-button[open]:empty::before,
 .devtools-toolbarbutton:not([label]):hover > image,
 .devtools-toolbarbutton:not([label])[checked=true] > image,
 .devtools-toolbarbutton:not([label])[open=true] > image {
   opacity: 1;
 }
 
 .devtools-button:disabled,
 .devtools-button[disabled],
 .devtools-toolbarbutton[disabled] {
+  pointer-events: none;
   opacity: 0.5 !important;
 }
 
 .devtools-button[checked]:empty::before,
 .devtools-button[open]:empty::before,
 .devtools-button.checked::before,
 .devtools-toolbarbutton:not([label])[checked=true] > image,
 .devtools-toolbarbutton:not([label])[open=true] > image {
@@ -392,39 +394,41 @@ checkbox:-moz-focusring {
 /* Icon-and-text buttons */
 .devtools-toolbarbutton.icon-and-text .toolbarbutton-text {
   margin-inline-start: .5em !important;
   font-weight: 600;
 }
 
 /* Text-only buttons */
 .theme-light .devtools-toolbarbutton[label]:not([text-as-image]):not([type=menu-button]),
-.theme-light .devtools-toolbarbutton[data-text-only] {
+.theme-light .devtools-toolbarbutton[data-text-only],
+.theme-light .devtools-button:not(:empty) {
   background-color: var(--toolbar-tab-hover);
 }
 .theme-dark .devtools-toolbarbutton[label]:not([text-as-image]):not([type=menu-button]),
-.theme-dark .devtools-toolbarbutton[data-text-only] {
+.theme-dark .devtools-toolbarbutton[data-text-only],
+.theme-dark .devtools-button:not(:empty) {
   background-color: rgba(0, 0, 0, .2); /* Splitter */
 }
 
 /* Text-only button states */
-.theme-dark .devtools-button:not(:empty):not([disabled]):hover,
+.theme-dark .devtools-button:not(:empty):not(:disabled):hover,
 .theme-dark .devtools-toolbarbutton:not(:-moz-any([checked=true],[disabled],[text-as-image]))[label]:hover {
   background: rgba(0, 0, 0, .3); /* Splitters */
 }
-.theme-light .devtools-button:not(:empty):not([disabled]):hover,
+.theme-light .devtools-button:not(:empty):not(:disabled):hover,
 .theme-light .devtools-toolbarbutton:not(:-moz-any([checked=true],[disabled],[text-as-image]))[label]:hover {
   background: rgba(170, 170, 170, .3); /* Splitters */
 }
 
-.theme-dark .devtools-button:not(:empty):not([disabled]):hover:active,
+.theme-dark .devtools-button:not(:empty):not(:disabled):hover:active,
 .theme-dark .devtools-toolbarbutton:not(:-moz-any([checked=true],[disabled],[text-as-image]))[label]:hover:active {
   background: rgba(0, 0, 0, .4); /* Splitters */
 }
-.theme-light .devtools-button:not(:empty):not([disabled]):hover:active,
+.theme-light .devtools-button:not(:empty):not(:disabled):hover:active,
 .theme-light .devtools-toolbarbutton:not(:-moz-any([checked=true],[disabled],[text-as-image]))[label]:hover:active {
   background: var(--toolbar-tab-hover-active);
 }
 
 .theme-dark .devtools-toolbarbutton:not([disabled])[label][checked=true],
 .theme-dark .devtools-toolbarbutton:not([disabled])[label][open],
 .theme-dark .devtools-button:not(:empty)[checked=true] {
   background: var(--theme-selection-background-semitransparent);
--- a/devtools/client/themes/performance.css
+++ b/devtools/client/themes/performance.css
@@ -111,27 +111,22 @@
 #main-record-button .button-text, #import-button .button-text {
   display: none;
 }
 
 .notice-container .record-button {
   padding: 5px !important;
 }
 
-.notice-container .record-button[checked],
-.notice-container .record-button[checked] {
+.notice-container .record-button.checked,
+.notice-container .record-button.checked {
   color: var(--theme-selection-color) !important;
   background: var(--theme-selection-background) !important;
 }
 
-.record-button[locked] {
-  pointer-events: none;
-  opacity: 0.5;
-}
-
 /* Sidebar & recording items */
 
 #recordings-pane {
   border-inline-end: 1px solid var(--theme-splitter-color);
   width: var(--sidebar-width);
 }
 
 #recording-controls-mount {