Profiler UI/UX tweaks and bug-fixes (#1112)

* Incorporated some interactions feedback from Shirley Wu:
1. Replace interaction commit circles with squares
2. Color commits in both the main and right-side pane based on their duration (to tie into other chart views)
3. Remove click-to-select commit because this surprised Sophie and Shirley initially

* Replaced simple cache with lru cache for chart data

* Only scroll selected node if not already visible

* Profiler radio button text is not selectable

* Don't persist selected chart type between sessions

* Removed hpyhen from interaction-tracking in 'learn more' note

* Fixed off-by-1 toolbar height issue

* Color selected commit black in interactions detail panel too

* Improve UI/UX for commits with a duration of 0

Author: bvaughn

frontend/Node.js

@@ -157,7 +157,9 @@ class Node extends React.Component<PropsType, StateType> {
   ensureInView() {
     var node = this.props.isBottomTagSelected ? this._tail : this._head;
     if (node != null) {
-      if (typeof node.scrollIntoView === 'function') {
+      if (typeof node.scrollIntoViewIfNeeded === 'function') {
+        node.scrollIntoViewIfNeeded();
+      } else if (typeof node.scrollIntoView === 'function') {
         node.scrollIntoView({
           // $FlowFixMe Flow does not realize block:"nearest" is a valid option
           block: 'nearest',
@@ -167,6 +169,13 @@ class Node extends React.Component<PropsType, StateType> {
     }
   }
 
+  _setTailRef = tail => {
+    this._tail = tail;
+  };
+  _setHeadRef = head => {
+    this._head = head;
+  };
+
   render() {
     const {theme} = this.context;
     const {
@@ -251,7 +260,7 @@ class Node extends React.Component<PropsType, StateType> {
       }
       return (
         <div
-          ref={h => this._head = h}
+          ref={this._setHeadRef}
           style={sharedHeadStyle}
           {...headEvents}
         >
@@ -293,9 +302,9 @@ class Node extends React.Component<PropsType, StateType> {
       const isCollapsed = content === null || content === undefined;
       return (
         <div style={headWrapperStyle}>
-          <div ref={h => this._head = h} style={sharedHeadStyle} {...headEvents}>
+          <div style={sharedHeadStyle} {...headEvents}>
             &lt;
-            <span style={jsxSingleLineTagStyle}>{name}</span>
+            <span ref={this._setHeadRef} style={jsxSingleLineTagStyle}>{name}</span>
             {node.get('key') &&
               <Props key="key" props={{'key': node.get('key')}} inverted={inverted}/>
             }
@@ -323,7 +332,7 @@ class Node extends React.Component<PropsType, StateType> {
     const closeTag = (
       <Fragment>
         &lt;/
-        <span style={jsxCloseTagStyle}>{name}</span>
+        <span ref={this._setTailRef} style={jsxCloseTagStyle}>{name}</span>
         &gt;
         {selected && ((collapsed && !this.props.isBottomTagSelected) || this.props.isBottomTagSelected) &&
           <span style={dollarRStyle}>&nbsp;== $r</span>
@@ -335,7 +344,7 @@ class Node extends React.Component<PropsType, StateType> {
 
     const jsxOpenTagStyle = jsxTagStyle(inverted && (!isBottomTagSelected || collapsed), nodeType, theme);
     const head = (
-      <div ref={h => this._head = h} style={sharedHeadStyle} {...headEvents}>
+      <div style={sharedHeadStyle} {...headEvents}>
         <span
           onClick={onToggleCollapse}
           style={{
@@ -347,7 +356,7 @@ class Node extends React.Component<PropsType, StateType> {
           {collapsed ? '▶' : '▼'}
         </span>
         &lt;
-        <span style={jsxOpenTagStyle}>{name}</span>
+        <span ref={this._setHeadRef} style={jsxOpenTagStyle}>{name}</span>
         {node.get('key') &&
           <Props key="key" props={{'key': node.get('key')}} inverted={headInverted}/>
         }
@@ -394,7 +403,7 @@ class Node extends React.Component<PropsType, StateType> {
         }}>
           {children.map(id => <WrappedNode key={id} depth={depth + 1} id={id}/>)}
         </div>
-        <div ref={t => this._tail = t} style={tailStyleActual} {...tailEvents}>
+        <div style={tailStyleActual} {...tailEvents}>
           {closeTag}
         </div>
       </div>

package.json

@@ -24,6 +24,7 @@
     "jest": "22.1.4",
     "json-loader": "0.5.4",
     "log-update": "^2.0.0",
+    "lru-cache": "^4.1.3",
     "memoize-one": "^3.1.1",
     "node-libs-browser": "0.5.3",
     "nullthrows": "^1.0.0",

plugins/Profiler/ProfilerStore.js

@@ -16,8 +16,8 @@ import type {ChartType, Interaction, RootProfilerData, Snapshot} from './Profile
 const {List} = require('immutable');
 const {EventEmitter} = require('events');
 const {get, set} = require('../../utils/storage');
+const LRU = require('lru-cache');
 
-const LOCAL_STORAGE_CHART_TYPE_KEY = 'profiler:selectedChartType';
 const LOCAL_STORAGE_COMMIT_THRESHOLD = 'profiler:commitThreshold';
 const LOCAL_STORAGE_HIDE_COMMITS_BELOW_THRESHOLD = 'profiler:hideCommitsBelowThreshold';
 const LOCAL_STORAGE_SHOW_NATIVE_NODES_KEY = 'profiler:showNativeNodes';
@@ -26,15 +26,15 @@ class ProfilerStore extends EventEmitter {
   _bridge: Bridge;
   _mainStore: Object;
 
-  cachedData = {};
+  cachedData = LRU(50); // Evict items from the cache after this number
   commitThreshold: number = ((get(LOCAL_STORAGE_COMMIT_THRESHOLD, 0): any): number);
   hideCommitsBelowThreshold: boolean = ((get(LOCAL_STORAGE_HIDE_COMMITS_BELOW_THRESHOLD, false): any): boolean);
   isRecording: boolean = false;
   isSettingsPanelActive: boolean = false;
   processedInteractions: {[id: string]: Interaction} = {};
   rootsToProfilerData: Map<string, RootProfilerData> = new Map();
   roots: List = new List();
-  selectedChartType: ChartType = ((get(LOCAL_STORAGE_CHART_TYPE_KEY, 'flamegraph'): any): ChartType);
+  selectedChartType: ChartType = 'flamegraph';
   selectedRoot: string | null = null;
   showNativeNodes: boolean = ((get(LOCAL_STORAGE_SHOW_NATIVE_NODES_KEY, false): any): boolean);
 
@@ -54,26 +54,26 @@ class ProfilerStore extends EventEmitter {
   }
 
   cacheDataForSnapshot(snapshotIndex: number, snapshotRootID: string, key: string, data: any): void {
-    this.cachedData[`${snapshotIndex}-${snapshotRootID}-${key}`] = data;
+    this.cachedData.set(`${snapshotIndex}-${snapshotRootID}-${key}`, data);
   }
 
   cacheInteractionData(rootID: string, data: any): void {
-    this.cachedData[`${rootID}-interactions`] = data;
+    this.cachedData.set(`${rootID}-interactions`, data);
   }
 
   clearSnapshots = () => {
-    this.cachedData = {};
+    this.cachedData.reset();
     this.processedInteractions = {};
     this.rootsToProfilerData = new Map();
     this.emit('profilerData', this.rootsToProfilerData);
   };
 
   getCachedDataForSnapshot(snapshotIndex: number, snapshotRootID: string, key: string): any {
-    return this.cachedData[`${snapshotIndex}-${snapshotRootID}-${key}`] || null;
+    return this.cachedData.get(`${snapshotIndex}-${snapshotRootID}-${key}`) || null;
   }
 
   getCachedInteractionData(rootID: string): any {
-    return this.cachedData[`${rootID}-interactions`] || null;
+    return this.cachedData.get(`${rootID}-interactions`) || null;
   }
 
   processInteraction(interaction: Interaction): Interaction {
@@ -117,7 +117,6 @@ class ProfilerStore extends EventEmitter {
   setSelectedChartType(selectedChartType: ChartType) {
     this.selectedChartType = selectedChartType;
     this.emit('selectedChartType', selectedChartType);
-    set(LOCAL_STORAGE_CHART_TYPE_KEY, selectedChartType);
   }
 
   setShowNativeNodes(showNativeNodes: boolean) {

plugins/Profiler/views/FiberRenderDurations.js

@@ -36,7 +36,7 @@ type ChartData = {|
 type ItemData = {|
   height: number,
   nodes: Array<Node>,
-  scaleY: (value: number) => number,
+  scaleY: (value: number, fallbackValue: number) => number,
   selectedSnapshot: Snapshot,
   selectSnapshot: SelectSnapshot,
   stopInspecting: Function,
@@ -179,7 +179,7 @@ class ListItem extends PureComponent<any, void> {
     const { height, nodes, scaleY, selectedSnapshot, selectSnapshot, stopInspecting, theme } = itemData;
 
     const node = nodes[index];
-    const safeHeight = Math.max(minBarHeight, scaleY(node.value));
+    const safeHeight = Math.max(minBarHeight, scaleY(node.value, minBarHeight));
 
     // List items are absolutely positioned using the CSS "left" attribute.
     // The "top" value will always be 0.