Skip to content

Commit 28a4e43

Browse files
committed
Address review comments
1 parent 0ec5385 commit 28a4e43

File tree

2 files changed

+28
-14
lines changed

2 files changed

+28
-14
lines changed

site/static/compare.html

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
262262
</span>
263263
</span>
264264
</div>
265-
<input type="checkbox" v-model="filter.showNonRelevant" style="margin-left: 20px;" />
265+
<input type="checkbox" v-model="filter.nonRelevant" style="margin-left: 20px;" />
266266
</div>
267267
<div class="section">
268268
<div class="section-heading"><span>Display raw data</span>
@@ -285,7 +285,8 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
285285
<span class="tooltip" style="margin-right: 1em;">?
286286
<span class="tooltiptext">
287287
The table shows summaries of regressions, improvements and all changes
288-
amongst the filtered benchmarks.
288+
calculated from data that is currently visible (data that passes the
289+
active filters).
289290
</span>
290291
</span>
291292
</div>

site/static/compare/script.js

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ function findQueryParam(name) {
99
function createDefaultFilter() {
1010
return {
1111
name: null,
12-
showNonRelevant: false,
12+
nonRelevant: false,
1313
profile: {
1414
check: true,
1515
debug: true,
@@ -107,7 +107,7 @@ const app = Vue.createApp({
107107
let nameFilter = filter.name && filter.name.trim();
108108
nameFilter = !nameFilter || name.includes(nameFilter);
109109

110-
const relevanceFilter = filter.showNonRelevant ? true : testCase.isRelevant;
110+
const relevanceFilter = filter.nonRelevant ? true : testCase.isRelevant;
111111

112112
return (
113113
profileFilter(testCase.profile) &&
@@ -391,18 +391,34 @@ app.component('test-cases-table', {
391391
const SummaryPercentValue = {
392392
props: {
393393
value: Number,
394+
padWidth: {
395+
type: Number,
396+
default: null
397+
}
394398
},
395399
template: `
396-
<span>{{ value.toFixed(2) }}%</span>
397-
`
400+
<span><span v-html="padSpaces" />{{ formattedValue }}%</span>
401+
`,
402+
computed: {
403+
formattedValue() {
404+
return `${this.signIfPositive(this.value)}${this.value.toFixed(2)}`;
405+
},
406+
padSpaces() {
407+
let value = this.formattedValue;
408+
if (value.length < this.padWidth) {
409+
return "&nbsp;".repeat(this.padWidth - value.length);
410+
}
411+
return "";
412+
}
413+
}
398414
};
399415
const SummaryRange = {
400416
props: {
401417
range: Array,
402418
},
403419
template: `
404420
<div v-if="range.length > 0">
405-
[<SummaryPercentValue :value="range[0]" />, <SummaryPercentValue :value="range[1]" />]
421+
[<SummaryPercentValue :value="range[0]" :padWidth="6" />, <SummaryPercentValue :value="range[1]" :padWidth="6" />]
406422
</div>
407423
<div v-else>-</div>
408424
`, components: {SummaryPercentValue}
@@ -422,29 +438,29 @@ app.component('summary-table', {
422438
template: `
423439
<table class="summary-table">
424440
<thead>
441+
<th><!-- icon --></th>
425442
<th>Range</th>
426443
<th>Mean</th>
427444
<th>Count</th>
428-
<th><!-- icon --></th>
429445
</thead>
430446
<tbody>
431447
<tr class="positive">
448+
<td title="Regresions">❌</td>
432449
<td><SummaryRange :range="cases.regressions.range" /></td>
433450
<td><SummaryPercentValue :value="cases.regressions.average" /></td>
434451
<td><SummaryCount :cases="cases.regressions.count" :benchmarks="cases.regressions.benchmarks" /></td>
435-
<td title="Regresions">❌</td>
436452
</tr>
437453
<tr class="negative">
454+
<td title="Improvements">✅</td>
438455
<td><SummaryRange :range="cases.improvements.range" /></td>
439456
<td><SummaryPercentValue :value="cases.improvements.average" /></td>
440457
<td><SummaryCount :cases="cases.improvements.count" :benchmarks="cases.improvements.benchmarks" /></td>
441-
<td title="Improvements">✅</td>
442458
</tr>
443459
<tr>
460+
<td title="All changes">✅,❌</td>
444461
<td><SummaryRange :range="cases.all.range" /></td>
445462
<td><SummaryPercentValue :value="cases.all.average" /></td>
446463
<td><SummaryCount :cases="cases.all.count" :benchmarks="cases.all.benchmarks" /></td>
447-
<td></td>
448464
</tr>
449465
</tbody>
450466
</table>
@@ -537,9 +553,6 @@ function computeSummary(testCases) {
537553
Math.min.apply(null, values),
538554
Math.max.apply(null, values),
539555
];
540-
if (Math.abs(range[0]) > Math.abs(range[1])) {
541-
range = [range[1], range[0]];
542-
}
543556
}
544557

545558
const sum = values.reduce((acc, item) => acc + item, 0);

0 commit comments

Comments
 (0)