The rows in the analysis results table should be expandable
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Feb 2016 02:44:34 +0000 (02:44 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Feb 2016 02:44:34 +0000 (02:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154427

Reviewed by Chris Dumez.

Added "(Expand)" link between rows that have hidden points. Upon click it inserts the hidden rows.

We insert around five rows at a time when there are hundreds of hidden points but we also avoid leaving
behind expandable rows of less than two rows.

Also fixed a bug in CustomizableTestGroupForm that getElementsById would throw in the shipping Safari
because getElementsById doesn't exist on Element.prototype by using class name instead.

* public/v3/components/analysis-results-viewer.js:
(AnalysisResultsViewer):
(AnalysisResultsViewer.prototype.setCurrentTestGroup): Removed superfluous call to render().
(AnalysisResultsViewer.prototype.setPoints): Always show the start and the end points.
(AnalysisResultsViewer.prototype.buildRowGroups):
(AnalysisResultsViewer.prototype._buildRowsForPointsAndTestGroups): Add an instance of ExpandableRow which
shows a "(Expand)" link to show hidden rows here.
(AnalysisResultsViewer.prototype._expandBetween): Added. Expands rows between two points.
(AnalysisResultsViewer.cssTemplate): Added rules for "(Expand)" links.
(AnalysisResultsViewer.ExpandableRow): Added.
(AnalysisResultsViewer.ExpandableRow.prototype.resultContent): Added. Overrides what's in the results column.
(AnalysisResultsViewer.ExpandableRow.prototype.heading): Added. Generates "(Expand)" link.

* public/v3/components/customizable-test-group-form.js:
(CustomizableTestGroupForm.prototype._computeRootSetMap): Use getElementsByClassName instead of
getElementById.
(CustomizableTestGroupForm.prototype._classForLabelAndRepository): Renamed from _idForLabelAndRepository.
(CustomizableTestGroupForm._constructRevisionRadioButtons): Set class name instead of id.

* public/v3/components/results-table.js:
(ResultsTable.prototype.render): Don't generate radio buttons to select a row when root set is missing;
e.g. for rows that show "(Expand)" links.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@196792 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/v3/components/analysis-results-viewer.js
Websites/perf.webkit.org/public/v3/components/customizable-test-group-form.js
Websites/perf.webkit.org/public/v3/components/results-table.js

index 4659aad..3f1468b 100644 (file)
@@ -1,5 +1,43 @@
 2016-02-18  Ryosuke Niwa  <rniwa@webkit.org>
 
+        The rows in the analysis results table should be expandable
+        https://bugs.webkit.org/show_bug.cgi?id=154427
+
+        Reviewed by Chris Dumez.
+
+        Added "(Expand)" link between rows that have hidden points. Upon click it inserts the hidden rows.
+
+        We insert around five rows at a time when there are hundreds of hidden points but we also avoid leaving
+        behind expandable rows of less than two rows.
+
+        Also fixed a bug in CustomizableTestGroupForm that getElementsById would throw in the shipping Safari
+        because getElementsById doesn't exist on Element.prototype by using class name instead.
+
+        * public/v3/components/analysis-results-viewer.js:
+        (AnalysisResultsViewer):
+        (AnalysisResultsViewer.prototype.setCurrentTestGroup): Removed superfluous call to render().
+        (AnalysisResultsViewer.prototype.setPoints): Always show the start and the end points.
+        (AnalysisResultsViewer.prototype.buildRowGroups):
+        (AnalysisResultsViewer.prototype._buildRowsForPointsAndTestGroups): Add an instance of ExpandableRow which
+        shows a "(Expand)" link to show hidden rows here.
+        (AnalysisResultsViewer.prototype._expandBetween): Added. Expands rows between two points.
+        (AnalysisResultsViewer.cssTemplate): Added rules for "(Expand)" links.
+        (AnalysisResultsViewer.ExpandableRow): Added.
+        (AnalysisResultsViewer.ExpandableRow.prototype.resultContent): Added. Overrides what's in the results column.
+        (AnalysisResultsViewer.ExpandableRow.prototype.heading): Added. Generates "(Expand)" link.
+
+        * public/v3/components/customizable-test-group-form.js:
+        (CustomizableTestGroupForm.prototype._computeRootSetMap): Use getElementsByClassName instead of
+        getElementById.
+        (CustomizableTestGroupForm.prototype._classForLabelAndRepository): Renamed from _idForLabelAndRepository.
+        (CustomizableTestGroupForm._constructRevisionRadioButtons): Set class name instead of id.
+
+        * public/v3/components/results-table.js:
+        (ResultsTable.prototype.render): Don't generate radio buttons to select a row when root set is missing;
+        e.g. for rows that show "(Expand)" links.
+
+2016-02-18  Ryosuke Niwa  <rniwa@webkit.org>
+
         Statistically significant A/B testing results should be color coded in details view
         https://bugs.webkit.org/show_bug.cgi?id=154414
 
index cf032e0..bbca73d 100644 (file)
@@ -11,6 +11,7 @@ class AnalysisResultsViewer extends ResultsTable {
         this._shouldRenderTable = true;
         this._additionalHeading = null;
         this._testGroupCallback = null;
+        this._expandedPoints = new Set;
     }
 
     setTestGroupCallback(callback) { this._testGroupCallback = callback; }
@@ -18,7 +19,6 @@ class AnalysisResultsViewer extends ResultsTable {
     setCurrentTestGroup(testGroup)
     {
         this._currentTestGroup = testGroup;
-        this.render();
     }
 
     setPoints(startPoint, endPoint)
@@ -26,6 +26,9 @@ class AnalysisResultsViewer extends ResultsTable {
         this._startPoint = startPoint;
         this._endPoint = endPoint;
         this._shouldRenderTable = true;
+        this._expandedPoints.clear();
+        this._expandedPoints.add(startPoint);
+        this._expandedPoints.add(endPoint);
     }
 
     setTestGroups(testGroups)
@@ -91,6 +94,11 @@ class AnalysisResultsViewer extends ResultsTable {
         var self = this;
         rowList.forEach(function (row, rowIndex) {
             var matchingRootSets = rowToMatchingRootSets.get(row);
+            if (!matchingRootSets) {
+                console.assert(row instanceof AnalysisResultsViewer.ExpandableRow);
+                return;
+            }
+
             for (var entry of matchingRootSets) {
                 var testGroup = entry.testGroup();
 
@@ -144,6 +152,7 @@ class AnalysisResultsViewer extends ResultsTable {
         var pointAfterEnd = this._endPoint.series.nextPoint(this._endPoint);
         var rootSetsWithPoints = new Set;
         var pointIndex = 0;
+        var previousPoint;
         for (var point = this._startPoint; point && point != pointAfterEnd; point = point.series.nextPoint(point), pointIndex++) {
             var rootSetInPoint = point.rootSet();
             var matchingRootSets = [];
@@ -154,12 +163,17 @@ class AnalysisResultsViewer extends ResultsTable {
                 }
             }
 
-            if (!matchingRootSets.length && point != this._startPoint && point != this._endPoint)
+            var hasMatchingTestGroup = !!matchingRootSets.length;
+            if (!hasMatchingTestGroup && !this._expandedPoints.has(point))
                 continue;
 
             var row = new ResultsTableRow(pointIndex.toString(), rootSetInPoint);
             row.setResult(point);
 
+            if (previousPoint && previousPoint.series.nextPoint(previousPoint) != point)
+                rowList.push(new AnalysisResultsViewer.ExpandableRow(this._expandBetween.bind(this, previousPoint, point)));
+            previousPoint = point;
+
             rowToMatchingRootSets.set(row, matchingRootSets);
             rowList.push(row);
         }
@@ -170,7 +184,7 @@ class AnalysisResultsViewer extends ResultsTable {
 
             for (var i = 0; i < rowList.length; i++) {
                 var row = rowList[i];
-                if (row.rootSet().equals(entry.rootSet())) {
+                if (!(row instanceof AnalysisResultsViewer.ExpandableRow) && row.rootSet().equals(entry.rootSet())) {
                     rowToMatchingRootSets.get(row).push(entry);
                     return;
                 }
@@ -178,6 +192,9 @@ class AnalysisResultsViewer extends ResultsTable {
 
             var groupTime = entry.rootSet().latestCommitTime();
             for (var i = 0; i < rowList.length; i++) {
+                if (rowList[i] instanceof AnalysisResultsViewer.ExpandableRow)
+                    continue;
+
                 var rowTime = rowList[i].rootSet().latestCommitTime();
                 if (rowTime > groupTime) {
                     var newRow = new ResultsTableRow(null, entry.rootSet());
@@ -190,6 +207,8 @@ class AnalysisResultsViewer extends ResultsTable {
                     // Missing some commits. Do as best as we can to avoid going backwards in time.
                     var repositoriesInNewRow = entry.rootSet().repositories();
                     for (var j = i; j < rowList.length; j++) {
+                        if (rowList[j] instanceof AnalysisResultsViewer.ExpandableRow)
+                            continue;
                         for (var repository of repositoriesInNewRow) {
                             var newCommit = entry.rootSet().commitForRepository(repository);
                             var rowCommit = rowList[j].rootSet().commitForRepository(repository);
@@ -222,6 +241,23 @@ class AnalysisResultsViewer extends ResultsTable {
         if (this._testGroupCallback)
             this._testGroupCallback(testGroup);
     }
+    
+    _expandBetween(pointBeforeExpansion, pointAfterExpansion)
+    {
+        console.assert(pointBeforeExpansion.series == pointAfterExpansion.series);
+        var indexBeforeStart = pointBeforeExpansion.seriesIndex;
+        var indexAfterEnd = pointAfterExpansion.seriesIndex;
+        console.assert(indexBeforeStart + 1 < indexAfterEnd);
+
+        var series = pointAfterExpansion.series;
+        var increment = Math.ceil((indexAfterEnd - indexBeforeStart) / 5);
+        if (increment < 3)
+            increment = 1;
+        for (var i = indexBeforeStart + 1; i < indexAfterEnd; i += increment)
+            this._expandedPoints.add(series.findPointByIndex(i));
+        this._shouldRenderTable = true;
+        this.render();
+    }
 
     static htmlTemplate()
     {
@@ -284,12 +320,37 @@ class AnalysisResultsViewer extends ResultsTable {
             .analysis-view .stacking-block.better {
                 background: rgba(102, 102, 255, 0.5);
             }
+
+            .analysis-view .point-label-with-expansion-link {
+                font-size: 0.7rem;
+            }
+            .analysis-view .point-label-with-expansion-link a {
+                color: #999;
+                text-decoration: none;
+            }
         `;
     }
 }
 
 ComponentBase.defineElement('analysis-results-viewer', AnalysisResultsViewer);
 
+AnalysisResultsViewer.ExpandableRow = class extends ResultsTableRow {
+    constructor(callback)
+    {
+        super(null, null);
+        this._callback = callback;
+    }
+
+    resultContent() { return ''; }
+
+    heading()
+    {
+        return ComponentBase.createElement('span', {class: 'point-label-with-expansion-link'}, [
+            ComponentBase.createLink('(Expand)', 'Expand', this._callback),
+        ]);
+    }
+}
+
 AnalysisResultsViewer.RootSetInTestGroup = class {
     constructor(testGroup, rootSet)
     {
index 4c209bd..e3e60ef 100644 (file)
@@ -42,8 +42,8 @@ class CustomizableTestGroupForm extends TestGroupForm {
         for (var label in this._rootSetMap) {
             var customRootSet = new CustomRootSet;
             for (var repository of this._renderedRepositorylist) {
-                var id = CustomizableTestGroupForm._idForLabelAndRepository(label, repository);
-                var revision = this.content().getElementById(id).value;
+                var className = CustomizableTestGroupForm._classForLabelAndRepository(label, repository);
+                var revision = this.content().getElementsByClassName(className)[0].value;
                 console.assert(revision);
                 if (revision)
                     customRootSet.setRevisionForRepository(repository, revision);
@@ -92,14 +92,14 @@ class CustomizableTestGroupForm extends TestGroupForm {
                     }))]));
     }
 
-    static _idForLabelAndRepository(label, repository) { return label + '-' + repository.id(); }
+    static _classForLabelAndRepository(label, repository) { return label + '-' + repository.id(); }
 
     static _constructRevisionRadioButtons(rootSetMap, repository, rowLabel)
     {
-        var id = this._idForLabelAndRepository(rowLabel, repository);
-        var groupName = id + '-group';
+        var className = this._classForLabelAndRepository(rowLabel, repository);
+        var groupName = className + '-group';
         var element = ComponentBase.createElement;
-        var revisionEditor = element('input', {id: id});
+        var revisionEditor = element('input', {class: className});
 
         var nodes = [];
         for (var labelToChoose in rootSetMap) {
index 0c838f3..6a9c07a 100644 (file)
@@ -33,6 +33,8 @@ class ResultsTable extends ComponentBase {
         var extraRepositories = [];
         var repositoryList = this._computeRepositoryList(rowGroups, extraRepositories);
 
+        this._selectedRange = {};
+
         var barGraphGroup = new BarGraphGroup(this._valueFormatter);
         var element = ComponentBase.createElement;
         var self = this;
@@ -46,10 +48,14 @@ class ResultsTable extends ComponentBase {
                 var cells = [];
 
                 if (self._rangeSelectorLabels) {
-                    self._selectedRange = {};
-                    for (var label of self._rangeSelectorLabels)
-                        cells.push(element('td', element('input',
-                            {type: 'radio', name: label, onchange: self._rangeSelectorClicked.bind(self, label, row)})));
+                    for (var label of self._rangeSelectorLabels) {
+                        var content = '';
+                        if (row.rootSet()) {
+                            content = element('input',
+                                {type: 'radio', name: label, onchange: self._rangeSelectorClicked.bind(self, label, row)});
+                        }
+                        cells.push(element('td', content));
+                    }
                 }
 
                 if (groupHeading !== undefined && !rowIndex)