Bug 1532583 - Add scalars for copy options in Changes panel. r=pbro
authorRazvan Caliman <rcaliman@mozilla.com>
Tue, 05 Mar 2019 14:38:36 +0000
changeset 520457 3f01af64f1bb74cf43ded6a381ae4c2a625fcf9a
parent 520456 b25ed3efc1b313962d3d360db7cedf033dadd27e
child 520458 24e79b0a187fdff462d085bc50b1adcc269e3509
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1532583
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1532583 - Add scalars for copy options in Changes panel. r=pbro Track the number of times people use any of the export options from the Changes panel: - Copy All Changes button - Copy Rule button - Copy Rule context menu option - Copy Declaration context menu option Differential Revision: https://phabricator.services.mozilla.com/D22090
devtools/client/inspector/changes/ChangesContextMenu.js
devtools/client/inspector/changes/ChangesView.js
devtools/client/inspector/changes/components/ChangesApp.js
devtools/client/inspector/changes/constants.js
toolkit/components/telemetry/Scalars.yaml
--- a/devtools/client/inspector/changes/ChangesContextMenu.js
+++ b/devtools/client/inspector/changes/ChangesContextMenu.js
@@ -59,17 +59,17 @@ class ChangesContextMenu {
     const declEl = target.closest(".changes__declaration");
     const ruleEl = target.closest("[data-rule-id]");
     const ruleId = ruleEl ? ruleEl.dataset.ruleId : null;
 
     if (ruleId || declEl) {
       // Copy Rule option
       menu.append(new MenuItem({
         label: getStr("changes.contextmenu.copyRule"),
-        click: () => this._onCopyRule(ruleId),
+        click: () => this._onCopyRule(ruleId, true),
       }));
 
       // Copy Declaration option. Visible only if there is a declaration element target.
       menu.append(new MenuItem({
         label: getStr("changes.contextmenu.copyDeclaration"),
         click: () => this._onCopyDeclaration(declEl),
         visible: !!declEl,
       }));
--- a/devtools/client/inspector/changes/ChangesView.js
+++ b/devtools/client/inspector/changes/ChangesView.js
@@ -13,17 +13,21 @@ loader.lazyRequireGetter(this, "ChangesC
 loader.lazyRequireGetter(this, "clipboardHelper", "devtools/shared/platform/clipboard");
 
 const ChangesApp = createFactory(require("./components/ChangesApp"));
 const { getChangesStylesheet } = require("./selectors/changes");
 
 const {
   TELEMETRY_SCALAR_CONTEXTMENU,
   TELEMETRY_SCALAR_CONTEXTMENU_COPY,
+  TELEMETRY_SCALAR_CONTEXTMENU_COPY_DECLARATION,
+  TELEMETRY_SCALAR_CONTEXTMENU_COPY_RULE,
   TELEMETRY_SCALAR_COPY,
+  TELEMETRY_SCALAR_COPY_ALL_CHANGES,
+  TELEMETRY_SCALAR_COPY_RULE,
 } = require("./constants");
 
 const {
   resetChanges,
   trackChange,
 } = require("./actions/changes");
 
 class ChangesView {
@@ -108,16 +112,17 @@ class ChangesView {
   }
 
   /**
    * Handler for the "Copy All Changes" button. Simple wrapper that just calls
    * |this.copyChanges()| with no filters in order to trigger default operation.
    */
   copyAllChanges() {
     this.copyChanges();
+    this.telemetry.scalarAdd(TELEMETRY_SCALAR_COPY_ALL_CHANGES, 1);
   }
 
   /**
    * Handler for the "Copy Changes" option from the context menu.
    * Builds a CSS text with the aggregated changes and copies it to the clipboard.
    *
    * Optional rule and source ids can be used to filter the scope of the operation:
    * - if both a rule id and source id are provided, copy only the changes to the
@@ -155,30 +160,40 @@ class ChangesView {
    *        Host element of a CSS declaration rendered the Changes panel.
    */
   copyDeclaration(element) {
     const name = element.querySelector(".changes__declaration-name").textContent;
     const value = element.querySelector(".changes__declaration-value").textContent;
     const isRemoved = element.classList.contains("diff-remove");
     const text = isRemoved ? `/* ${name}: ${value}; */` : `${name}: ${value};`;
     clipboardHelper.copyString(text);
+    this.telemetry.scalarAdd(TELEMETRY_SCALAR_CONTEXTMENU_COPY_DECLARATION, 1);
   }
 
   /**
-   * Handler for the "Copy Rule" option from the context menu.
+   * Handler for the "Copy Rule" option from the context menu and "Copy Rule" button.
    * Gets the full content of the target CSS rule (including any changes applied)
    * and copies it to the clipboard.
    *
    * @param {String} ruleId
    *        Rule id of the target CSS rule.
+   * @param {Boolean} usingContextMenu
+   *        True if the handler is invoked from the context menu.
+   *        (Default) False if invoked from the button.
    */
-  async copyRule(ruleId) {
+  async copyRule(ruleId, usingContextMenu = false) {
     const rule = await this.inspector.pageStyle.getRule(ruleId);
     const text = await rule.getRuleText();
     clipboardHelper.copyString(text);
+
+    if (usingContextMenu) {
+      this.telemetry.scalarAdd(TELEMETRY_SCALAR_CONTEXTMENU_COPY_RULE, 1);
+    } else {
+      this.telemetry.scalarAdd(TELEMETRY_SCALAR_COPY_RULE, 1);
+    }
   }
 
   /**
    * Handler for the "Copy" option from the context menu.
    * Copies the current text selection to the clipboard.
    */
   copySelection() {
     clipboardHelper.copyString(this.window.getSelection().toString());
--- a/devtools/client/inspector/changes/components/ChangesApp.js
+++ b/devtools/client/inspector/changes/components/ChangesApp.js
@@ -33,28 +33,32 @@ class ChangesApp extends PureComponent {
   constructor(props) {
     super(props);
   }
 
   renderCopyAllChangesButton() {
     return dom.button(
       {
         className: "changes__copy-all-changes-button",
-        onClick: this.props.onCopyAllChanges,
+        onClick: e => {
+          e.stopPropagation();
+          this.props.onCopyAllChanges();
+        },
         title: getStr("changes.contextmenu.copyAllChangesDescription"),
       },
       getStr("changes.contextmenu.copyAllChanges")
     );
   }
 
   renderCopyButton(ruleId) {
     return dom.button(
       {
         className: "changes__copy-rule-button",
         onClick: e => {
+          e.stopPropagation();
           this.props.onCopyRule(ruleId);
         },
         title: getStr("changes.contextmenu.copyRuleDescription"),
       },
       getStr("changes.contextmenu.copyRule")
     );
   }
 
--- a/devtools/client/inspector/changes/constants.js
+++ b/devtools/client/inspector/changes/constants.js
@@ -4,10 +4,15 @@
  * 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";
 
 module.exports = {
   TELEMETRY_SCALAR_CONTEXTMENU: "devtools.changesview.contextmenu",
   TELEMETRY_SCALAR_CONTEXTMENU_COPY: "devtools.changesview.contextmenu_copy",
+  TELEMETRY_SCALAR_CONTEXTMENU_COPY_DECLARATION:
+  "devtools.changesview.contextmenu_copy_declaration",
+  TELEMETRY_SCALAR_CONTEXTMENU_COPY_RULE: "devtools.changesview.contextmenu_copy_rule",
   TELEMETRY_SCALAR_COPY: "devtools.changesview.copy",
+  TELEMETRY_SCALAR_COPY_ALL_CHANGES: "devtools.changesview.copy_all_changes",
+  TELEMETRY_SCALAR_COPY_RULE: "devtools.changesview.copy_rule",
 };
--- a/toolkit/components/telemetry/Scalars.yaml
+++ b/toolkit/components/telemetry/Scalars.yaml
@@ -1654,16 +1654,44 @@ devtools.changesview:
     kind: uint
     notification_emails:
       - dev-developer-tools@lists.mozilla.org
       - mbalfanz@mozilla.com
     release_channel_collection: opt-out
     record_in_processes:
       - 'main'
 
+  copy_all_changes:
+    bug_numbers:
+      - 1532583
+    description: >
+      Number of times the Copy All Changes button is used in the Changes panel.
+    expires: "70"
+    kind: uint
+    notification_emails:
+      - dev-developer-tools@lists.mozilla.org
+      - mbalfanz@mozilla.com
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
+  copy_rule:
+    bug_numbers:
+      - 1532583
+    description: >
+      Number of times the Copy Rule button is used in the Changes panel.
+    expires: "70"
+    kind: uint
+    notification_emails:
+      - dev-developer-tools@lists.mozilla.org
+      - mbalfanz@mozilla.com
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
   contextmenu:
     bug_numbers:
       - 1522843
     description: >
       Number of times the Changes panel context menu is shown.
     expires: "69"
     kind: uint
     notification_emails:
@@ -1682,16 +1710,46 @@ devtools.changesview:
     kind: uint
     notification_emails:
       - dev-developer-tools@lists.mozilla.org
       - mbalfanz@mozilla.com
     release_channel_collection: opt-out
     record_in_processes:
       - 'main'
 
+  contextmenu_copy_declaration:
+    bug_numbers:
+      - 1532583
+    description: >
+      Number of times the Copy Declaration option is picked from the Changes panel
+      context menu.
+    expires: "70"
+    kind: uint
+    notification_emails:
+      - dev-developer-tools@lists.mozilla.org
+      - mbalfanz@mozilla.com
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
+  contextmenu_copy_rule:
+    bug_numbers:
+      - 1532583
+    description: >
+      Number of times the Copy Rule option is picked from the Changes panel
+      context menu.
+    expires: "70"
+    kind: uint
+    notification_emails:
+      - dev-developer-tools@lists.mozilla.org
+      - mbalfanz@mozilla.com
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
 navigator.storage:
   estimate_count:
     bug_numbers:
       - 1359708
     description: >
       Number of times navigator.storage.estimate has been used.
     expires: "60"
     kind: uint