From 8b1305be4bf8c8e496e3d79ff3d0876d5765b696 Mon Sep 17 00:00:00 2001 From: Greg Tatum Date: Tue, 26 May 2020 15:59:59 +0000 Subject: [PATCH] Bug 1634448 - Update the menu button to reflect the current profiler states; r=canaltinova This patch adjusts the profiler menu button to properly reflect the current state of the profiler. It doesn't completely match the design spec, as there are a bunch of CSS rules already in place in the toolbar, and I wanted to keep the changes simple. It does however, update the UI based on the state of the profiler. Differential Revision: https://phabricator.services.mozilla.com/D75851 --- .../themes/shared/toolbarbutton-icons.inc.css | 21 ++++- .../performance-new/popup/menu-button.jsm.js | 45 ++++++++--- .../performance-new/test/browser/browser.ini | 1 + .../browser/browser_popup-profiler-states.js | 81 +++++++++++++++++++ .../client/themes/images/tool-profiler.svg | 2 +- 5 files changed, 138 insertions(+), 12 deletions(-) create mode 100644 devtools/client/performance-new/test/browser/browser_popup-profiler-states.js diff --git a/browser/themes/shared/toolbarbutton-icons.inc.css b/browser/themes/shared/toolbarbutton-icons.inc.css index 9f80d9f3b7cd..5cedf8a8ddc6 100644 --- a/browser/themes/shared/toolbarbutton-icons.inc.css +++ b/browser/themes/shared/toolbarbutton-icons.inc.css @@ -257,7 +257,26 @@ toolbar[brighttext] { } #profiler-button { - list-style-image: url("chrome://devtools/skin/images/profiler-stopwatch.svg"); + list-style-image: url("chrome://devtools/skin/images/tool-profiler.svg"); + transform: scaleX(-1); +} + +#profiler-button.profiler-active { + transform: scaleX(1); + fill: #0060df; + fill-opacity: 1; +} + +#profiler-button.profiler-active image { + background-color: #0060df33; +} + +#profiler-button.profiler-active:hover image { + background-color: #0060df22; +} + +#profiler-button.profiler-paused image { + opacity: 0.4; } #preferences-button { diff --git a/devtools/client/performance-new/popup/menu-button.jsm.js b/devtools/client/performance-new/popup/menu-button.jsm.js index cbf89785bdf3..e3fa3f68a976 100644 --- a/devtools/client/performance-new/popup/menu-button.jsm.js +++ b/devtools/client/performance-new/popup/menu-button.jsm.js @@ -229,21 +229,46 @@ function initialize(toggleProfilerKeyShortcuts) { return; } - function updateButtonColor() { - // Use photon blue-60 when active. - buttonElement.style.fill = Services.profiler.IsActive() - ? "#0060df" - : ""; + function setButtonActive() { + buttonElement.setAttribute( + "tooltiptext", + "The profiler is recording a profile" + ); + buttonElement.classList.toggle("profiler-active", true); + buttonElement.classList.toggle("profiler-paused", false); + } + function setButtonPaused() { + buttonElement.setAttribute( + "tooltiptext", + "The profiler is capturing a profile" + ); + buttonElement.classList.toggle("profiler-active", false); + buttonElement.classList.toggle("profiler-paused", true); + } + function setButtonInactive() { + buttonElement.setAttribute( + "tooltiptext", + "Record a performance profile" + ); + buttonElement.classList.toggle("profiler-active", false); + buttonElement.classList.toggle("profiler-paused", false); } - updateButtonColor(); + if (Services.profiler.IsPaused()) { + setButtonPaused(); + } + if (Services.profiler.IsActive()) { + setButtonActive(); + } - Services.obs.addObserver(updateButtonColor, "profiler-started"); - Services.obs.addObserver(updateButtonColor, "profiler-stopped"); + Services.obs.addObserver(setButtonActive, "profiler-started"); + Services.obs.addObserver(setButtonInactive, "profiler-stopped"); + Services.obs.addObserver(setButtonPaused, "profiler-paused"); window.addEventListener("unload", () => { - Services.obs.removeObserver(updateButtonColor, "profiler-started"); - Services.obs.removeObserver(updateButtonColor, "profiler-stopped"); + Services.obs.removeObserver(setButtonActive, "profiler-started"); + Services.obs.removeObserver(setButtonInactive, "profiler-stopped"); + Services.obs.removeObserver(setButtonPaused, "profiler-paused"); }); }, }; diff --git a/devtools/client/performance-new/test/browser/browser.ini b/devtools/client/performance-new/test/browser/browser.ini index 1c1a501b1cc5..d6f85521b455 100644 --- a/devtools/client/performance-new/test/browser/browser.ini +++ b/devtools/client/performance-new/test/browser/browser.ini @@ -25,6 +25,7 @@ support-files = [browser_devtools-record-capture.js] [browser_devtools-record-discard.js] [browser_webchannel-enable-menu-button.js] +[browser_popup-profiler-states.js] [browser_popup-record-capture.js] [browser_popup-record-discard.js] [browser_popup-private-browsing.js] diff --git a/devtools/client/performance-new/test/browser/browser_popup-profiler-states.js b/devtools/client/performance-new/test/browser/browser_popup-profiler-states.js new file mode 100644 index 000000000000..a335e626f5da --- /dev/null +++ b/devtools/client/performance-new/test/browser/browser_popup-profiler-states.js @@ -0,0 +1,81 @@ +/* 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"; + +add_task(async function test() { + info( + "Test the states of the profiler button, e.g. inactive, active, and capturing." + ); + await setProfilerFrontendUrl( + "http://example.com/browser/devtools/client/performance-new/test/browser/fake-frontend.html" + ); + await makeSureProfilerPopupIsEnabled(); + + const { toggleProfiler, captureProfile } = ChromeUtils.import( + "resource://devtools/client/performance-new/popup/background.jsm.js" + ); + + const button = document.getElementById("profiler-button"); + if (!button) { + throw new Error("Could not find the profiler button."); + } + + info("The profiler button starts out inactive"); + checkButtonState(button, { + tooltip: "Record a performance profile", + active: false, + paused: false, + }); + + info("Toggling the profiler turns on the active state"); + toggleProfiler("aboutprofiling"); + checkButtonState(button, { + tooltip: "The profiler is recording a profile", + active: true, + paused: false, + }); + + info("Capturing a profile makes the button paused"); + captureProfile("aboutprofiling"); + checkButtonState(button, { + tooltip: "The profiler is capturing a profile", + active: false, + paused: true, + }); + + waitUntil( + () => !button.classList.contains("profiler-paused"), + "Waiting until the profiler is no longer paused" + ); + + await checkTabLoadedProfile({ + initialTitle: "Waiting on the profile", + successTitle: "Profile received", + errorTitle: "Error", + }); +}); + +/** + * This check dives into the implementation details of the button, mainly + * because it's hard to provide a user-focused interpretation of button + * stylings. + */ +function checkButtonState(button, { tooltip, active, paused }) { + is( + button.getAttribute("tooltiptext"), + tooltip, + `The tooltip for the button is "${tooltip}".` + ); + is( + button.classList.contains("profiler-active"), + active, + `The expected profiler button active state is: ${active}` + ); + is( + button.classList.contains("profiler-paused"), + paused, + `The expected profiler button paused state is: ${paused}` + ); +} diff --git a/devtools/client/themes/images/tool-profiler.svg b/devtools/client/themes/images/tool-profiler.svg index a6029f0d5548..44e435001ae5 100644 --- a/devtools/client/themes/images/tool-profiler.svg +++ b/devtools/client/themes/images/tool-profiler.svg @@ -1,6 +1,6 @@ - +