Bug 152311 – Add v3 UI to perf dashboard

• Component: Perf Dashboard
• Status: RESOLVED FIXED

[Ryosuke Niwa]

V3 UI is coming.

[Ryosuke Niwa]

Created attachment 267416 [details]
Adds v3 UI

[Ryosuke Niwa]

This new UI reduces the amount of data downloaded from 27MB to 3MB on our internal dashboard.

[Ryosuke Niwa]

Created attachment 267424 [details]
Minor tweak

[Chris Dumez]

Comment on attachment 267424 [details]
Minor tweak

View in context: https://bugs.webkit.org/attachment.cgi?id=267424&action=review

r=me with nits

> Websites/perf.webkit.org/public/v3/main.js:22
> +        var chartToolbar = new ChartsToolbar;

nit: chartsToolbar for consistency?

> Websites/perf.webkit.org/public/v3/components/chart-status-view.js:92
> +            this._comparisonClass =  status.className;

Nit: extra space

> Websites/perf.webkit.org/public/v3/components/time-series-chart.js:375
> +/*                var x1 = i ? (series[i - 1].x + point.x) / 2 : point.x;

Did you mean to keep this in?

> Websites/perf.webkit.org/public/v3/components/time-series-chart.js:678
> +/*console.log(TimeSeriesChart.computeTimeGrid(

Did you mean to keep this in?

> Websites/perf.webkit.org/public/v3/components/time-series-chart.js:691
> +/*

Did you mean to keep this in?

> Websites/perf.webkit.org/public/v3/models/analysis-task.js:104
> +        // FIXME: The backend shouldn't create a separate bug rows per task for the same bug number.

"row" ?

> Websites/perf.webkit.org/public/v3/pages/analysis-category-toolbar.js:74
> +            this._filterCallback(this._filter);

this._filter is always null here is this intended?

> Websites/perf.webkit.org/public/v3/pages/chart-pane-status-view.js:37
> +            // selected ? '\u25B8' : '\u25B4'

did you mean to keep this in?

> Websites/perf.webkit.org/public/v3/pages/dashboard-page.js:12
> +        this._startTime = new Date(2015, 10, 19, 0, 0, 0) - 60 * 24 * 3600 * 1000;

these hardcoded dates seem weird

> Websites/perf.webkit.org/public/v3/pages/dashboard-page.js:13
> +        this._endTime = new Date(2015, 10, 19, 0, 0, 0);

ditto

> Websites/perf.webkit.org/public/v3/pages/heading.js:12
> +

blank line

> Websites/perf.webkit.org/public/v3/pages/heading.js:176
> +/*

Did you mean to keep this in?

> Websites/perf.webkit.org/tools/bundle-v3-scripts.py:26
> +    minidifed_script = jsmin.communicate(input=bundled_script)[0]

minified_script?

[Ryosuke Niwa]

Comment on attachment 267424 [details]
Minor tweak

View in context: https://bugs.webkit.org/attachment.cgi?id=267424&action=review

Thanks for the review!

>> Websites/perf.webkit.org/public/v3/main.js:22
>> +        var chartToolbar = new ChartsToolbar;
> 
> nit: chartsToolbar for consistency?

Fixed

>> Websites/perf.webkit.org/public/v3/components/chart-status-view.js:92
>> +            this._comparisonClass =  status.className;
> 
> Nit: extra space

Fixed.

>> Websites/perf.webkit.org/public/v3/components/time-series-chart.js:375
>> +/*                var x1 = i ? (series[i - 1].x + point.x) / 2 : point.x;
> 
> Did you mean to keep this in?

Removed.

>> Websites/perf.webkit.org/public/v3/components/time-series-chart.js:678
>> +/*console.log(TimeSeriesChart.computeTimeGrid(
> 
> Did you mean to keep this in?

Removed.

>> Websites/perf.webkit.org/public/v3/components/time-series-chart.js:691
>> +/*
> 
> Did you mean to keep this in?

Ditto.

>> Websites/perf.webkit.org/public/v3/models/analysis-task.js:104
>> +        // FIXME: The backend shouldn't create a separate bug rows per task for the same bug number.
> 
> "row" ?

Fixed.

>> Websites/perf.webkit.org/public/v3/pages/analysis-category-toolbar.js:74
>> +            this._filterCallback(this._filter);
> 
> this._filter is always null here is this intended?

Yes, this is intended. We clear the filter when we change the category.

>> Websites/perf.webkit.org/public/v3/pages/chart-pane-status-view.js:37
>> +            // selected ? '\u25B8' : '\u25B4'
> 
> did you mean to keep this in?

Removed.

>> Websites/perf.webkit.org/public/v3/pages/dashboard-page.js:12
>> +        this._startTime = new Date(2015, 10, 19, 0, 0, 0) - 60 * 24 * 3600 * 1000;
> 
> these hardcoded dates seem weird

Oops, this is supposed to be Date.now().  Fixed.

>> Websites/perf.webkit.org/public/v3/pages/dashboard-page.js:13
>> +        this._endTime = new Date(2015, 10, 19, 0, 0, 0);
> 
> ditto

Ditto.

>> Websites/perf.webkit.org/public/v3/pages/heading.js:12
>> +
> 
> blank line

Removed.

>> Websites/perf.webkit.org/public/v3/pages/heading.js:176
>> +/*
> 
> Did you mean to keep this in?

Removed.

>> Websites/perf.webkit.org/tools/bundle-v3-scripts.py:26
>> +    minidifed_script = jsmin.communicate(input=bundled_script)[0]
> 
> minified_script?

Fixed.

[Ryosuke Niwa]

Committed r194130: <http://trac.webkit.org/changeset/194130>