DevTools: track more elements panel user actions

DevTools: track more elements panel user actions

BUG=689289
Review-Url: https://codereview.chromium.org/2678233002
Cr-Commit-Position: refs/heads/master@{#455943}
Committed: https://chromium.googlesource.com/chromium/src/+/314767b924f73bc5e396cbb81973715af167a466

[chenwilliam, 2017-02-07 02:17:08]

chenwilliam@chromium.org changed reviewers:
+ pfeldman@chromium.org

[chenwilliam, 2017-02-07 02:17:09]

ptal

[pfeldman, 2017-02-07 19:15:27]

https://codereview.chromium.org/2678233002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js
(right):

https://codereview.chromium.org/2678233002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js:596:
Host.userMetrics.actionTaken(Host.UserMetrics.Action.ElementsTreeOutlineClick);
That's going to be comparable to the style edit.

https://codereview.chromium.org/2678233002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/elements/InspectElementModeController.js
(right):

https://codereview.chromium.org/2678233002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/elements/InspectElementModeController.js:123:
Host.userMetrics.actionTaken(Host.UserMetrics.Action.InspectElementModeToggleOn);
Are we after certain actionability here?

https://codereview.chromium.org/2678233002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeWrapper.js
(right):

https://codereview.chromium.org/2678233002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeWrapper.js:26:
Host.userMetrics.actionTaken(Host.UserMetrics.Action.DeviceModeToggleOn);
Wait, we had that, where did it go?

https://codereview.chromium.org/2678233002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/host/UserMetrics.js (right):

https://codereview.chromium.org/2678233002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/host/UserMetrics.js:75:
DeviceModeEnabled: 10,
Here it is!

[chenwilliam, 2017-02-08 01:07:00]

Description was changed from

==========
DevTools: track more device mode and elements user actions

BUG=689289
==========

to

==========
DevTools: track more  elements panel user actions

BUG=689289
==========

[chenwilliam, 2017-02-08 01:07:20]

Description was changed from

==========
DevTools: track more  elements panel user actions

BUG=689289
==========

to

==========
DevTools: track more elements panel user actions

BUG=689289
==========

[chenwilliam, 2017-02-08 01:10:21]

ptal

https://codereview.chromium.org/2678233002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js
(right):

https://codereview.chromium.org/2678233002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js:397:
if (!this._firstTimeSelectedNodeChanged)
It's hacky, but I don't want to include first-time use since many of those are
just people opening DevTools and don't reflect actual intent to inspect another
node.

It's in a similar vein to "this._notFirstInspectElement" in ElementsPanel.js

[pfeldman, 2017-02-08 19:04:48]

https://codereview.chromium.org/2678233002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js
(right):

https://codereview.chromium.org/2678233002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js:76:
this._firstTimeSelectedNodeChanged = true;
nit: typically you go from false to true. "this._reportedNodeSelectedUMA"

https://codereview.chromium.org/2678233002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js
(right):

https://codereview.chromium.org/2678233002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2091:
this.listItemElement.addEventListener('copy', this._clipboardCopy.bind(this));
You just created a lot of listeners for no good reason. Place one on the
sidebar's element instead.

[chenwilliam, 2017-02-10 22:53:37]

chenwilliam@chromium.org changed reviewers:
+ paulirish@chromium.org

[chenwilliam, 2017-02-10 22:53:38]

ptal

https://codereview.chromium.org/2678233002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js
(right):

https://codereview.chromium.org/2678233002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js:76:
this._firstTimeSelectedNodeChanged = true;
On 2017/02/08 at 19:04:48, pfeldman wrote:
> nit: typically you go from false to true. "this._reportedNodeSelectedUMA"

Done.

https://codereview.chromium.org/2678233002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js
(right):

https://codereview.chromium.org/2678233002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2091:
this.listItemElement.addEventListener('copy', this._clipboardCopy.bind(this));
On 2017/02/08 at 19:04:48, pfeldman wrote:
> You just created a lot of listeners for no good reason. Place one on the
sidebar's element instead.

Done.

https://codereview.chromium.org/2678233002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js
(right):

https://codereview.chromium.org/2678233002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:495:
// event.target is unreliable for copy event
w/o this hack, it was firing after I selected StylesSidebarPane and then tried
copying nodes on the ElementsTreeOutline.

[pfeldman, 2017-02-10 23:00:46]

https://codereview.chromium.org/2678233002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js
(right):

https://codereview.chromium.org/2678233002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js:397:
if (this._reportedNodeSelectedUMA)
!this._reportedNodeSelectedUMA

https://codereview.chromium.org/2678233002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js
(right):

https://codereview.chromium.org/2678233002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:495:
// event.target is unreliable for copy event
On 2017/02/10 22:53:38, chenwilliam wrote:
> w/o this hack, it was firing after I selected StylesSidebarPane and then tried
> copying nodes on the ElementsTreeOutline.

Can't be!

[pfeldman, 2017-02-23 01:13:21]

ping

[chenwilliam, 2017-02-25 00:49:15]

ptal

https://codereview.chromium.org/2678233002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js
(right):

https://codereview.chromium.org/2678233002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js:397:
if (this._reportedNodeSelectedUMA)
On 2017/02/10 23:00:46, pfeldman wrote:
> !this._reportedNodeSelectedUMA

I want to report it on the second time and afterwards, not the first time,
because the first time might be people using Inspect to open DevTools.

https://codereview.chromium.org/2678233002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js
(right):

https://codereview.chromium.org/2678233002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:495:
// event.target is unreliable for copy event
On 2017/02/10 23:00:46, pfeldman wrote:
> On 2017/02/10 22:53:38, chenwilliam wrote:
> > w/o this hack, it was firing after I selected StylesSidebarPane and then
tried
> > copying nodes on the ElementsTreeOutline.
> 
> Can't be!

Seems like the focus was fixed...

[pfeldman, 2017-02-25 01:34:36]

lgtm, but you don't need a boolean flag if you place instrumentation where i
suggest.

https://codereview.chromium.org/2678233002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js
(right):

https://codereview.chromium.org/2678233002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js:1690:
this._nodeShortcut.deferredNode.highlight();
This is a good place for manual selection tracking.

[chenwilliam, 2017-02-28 22:31:17]

@pfeldman, please take another look at the comments - it seems like
ElementsTreeOutline onselect is dead code

https://codereview.chromium.org/2678233002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js
(right):

https://codereview.chromium.org/2678233002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeOutline.js:1690:
this._nodeShortcut.deferredNode.highlight();
On 2017/02/25 01:34:36, pfeldman wrote:
> This is a good place for manual selection tracking.

I tried putting it here - but AFAICT it never gets triggered (both in manual
testing and our test suite). It seems that ElementsTreeElement onselect gets
triggered and not the Outline.

https://codereview.chromium.org/2678233002/diff/100001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeElement.js
(right):

https://codereview.chromium.org/2678233002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeElement.js:328:
Host.userMetrics.actionTaken(Host.UserMetrics.Action.ChangeInspectedNodeInElementsPanel);
This only gets triggered when you select from the elements tree outline, not
from when you use "inspect element".

[chenwilliam, 2017-03-09 22:27:59]

The CQ bit was checked by chenwilliam@chromium.org

[chenwilliam, 2017-03-09 22:27:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/2678233002/#ps120001
(title: "fixup")

[commit-bot: I haz the power, 2017-03-09 22:29:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-09 22:38:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-09 22:38:52]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[chenwilliam, 2017-03-09 23:22:59]

chenwilliam@chromium.org changed reviewers:
+ isherman@chromium.org

[chenwilliam, 2017-03-09 23:22:59]

@isherman, please take a look at histograms.xml, thanks.

[Ilya Sherman, 2017-03-10 00:42:53]

histograms.xml lgtm

[chenwilliam, 2017-03-10 01:07:20]

The CQ bit was checked by chenwilliam@chromium.org

[commit-bot: I haz the power, 2017-03-10 01:08:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-10 01:18:31]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1489108040973220,
"parent_rev": "624c5cfae4ce865f8ac91be865a29ff0e9fdbe34", "commit_rev":
"314767b924f73bc5e396cbb81973715af167a466"}

[commit-bot: I haz the power, 2017-03-10 01:19:56]

Description was changed from

==========
DevTools: track more elements panel user actions

BUG=689289
==========

to

==========
DevTools: track more elements panel user actions

BUG=689289

Review-Url: https://codereview.chromium.org/2678233002
Cr-Commit-Position: refs/heads/master@{#455943}
Committed:
https://chromium.googlesource.com/chromium/src/+/314767b924f73bc5e396cbb81973...
==========

[commit-bot: I haz the power, 2017-03-10 01:19:57]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/314767b924f73bc5e396cbb81973...