1336379 - Implement Statistics Panel component

Status: VERIFIED FIXED

(DevTools :: Netmonitor, defect, P1)

Description

[Ricky Chien [:rickychien] (inactive)]

Architecture reactify step for creating top level NetworkMonitor react component.

* Implement StatisticsPanel component instead of statistics-view.js
* Remove statistics-view.js
* Move whenDataAvailable mechanism to StatisticsPanel

Comment 1

[Ricky Chien [:rickychien] (inactive)]

Attachment: Bug 1336379 - Implement StatisticsPanel

Review commit: https://reviewboard.mozilla.org/r/109546/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/109546/

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8833290 [details]
Bug 1336379 - Implement StatisticsPanel

https://reviewboard.mozilla.org/r/109546/#review110612

I like it, just couple of comments.

R+ assuming you resolve my comments and try is green.

Thanks!
Honza

::: devtools/client/netmonitor/components/statistics-panel.js:33
(Diff revision 1)
> +const BACK_BUTTON = L10N.getStr("netmonitor.backButton");
> +const CHARTS_CACHE_ENABLED = L10N.getStr("charts.cacheEnabled");
> +const CHARTS_CACHE_DISABLED = L10N.getStr("charts.cacheDisabled");
> +
> +/*
> + * Statistics panel componennt

typo: componennt -> component

::: devtools/client/netmonitor/netmonitor-view.js
(Diff revision 1)
>    },
>  
> -  /**
> -   * Gets the current mode for this tool.
> -   * @return string (e.g, "network-inspector-view" or "network-statistics-view")
> -   */

Why did you remove the comments?

::: devtools/client/netmonitor/netmonitor-view.js
(Diff revision 1)
> -      clearInterval(interval);
> -      reject(new Error("Timed out while waiting for data"));
> -    }, WDA_DEFAULT_GIVE_UP_TIMEOUT);
> -  });
> -}
> -

Nice simplification! (code removal)

Comment 3

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8833290 [details]
Bug 1336379 - Implement StatisticsPanel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/109546/diff/1-2/

Comment 4

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8833290 [details]
Bug 1336379 - Implement StatisticsPanel

https://reviewboard.mozilla.org/r/109546/#review110612

> Why did you remove the comments?

Oops! It was a mistake when splitting the commit. But I'd like to drop this issue because this part (netmonitor-view.js) will be removed soon.

Comment 5

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8833290 [details]
Bug 1336379 - Implement StatisticsPanel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/109546/diff/2-3/

Comment 6

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8833290 [details]
Bug 1336379 - Implement StatisticsPanel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/109546/diff/3-4/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f0ca34823cf7
Implement StatisticsPanel r=Honza

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f0ca34823cf7

Comment 9

[Ciprian Georgiu [:ciprian_georgiu], Release Desktop QA]

I have performed tests around Statistics Panel on latest Nightly 54.0a1 (2017-02-09), and I can confirm that everything is working as expected across platforms: Windows 10 x64, Ubuntu 16.04 LTS and Mac OS X 10.11.6.

Marking this bug as verified fixed.