Change chrome://net-export/ UI flow.

Change chrome://net-export/ UI flow.

Instead of an all-in-one page displaying the current state and disabled
future states, splits the user journey among separate screens, each
with their own instructions.

BUG=701033
Review-Url: https://codereview.chromium.org/2850253002
Cr-Commit-Position: refs/heads/master@{#468783}
Committed: https://chromium.googlesource.com/chromium/src/+/912ec7d24049de83f175b212da8233edf78a30e2

[eroman, 2017-05-01 19:42:25]

eroman@chromium.org changed reviewers:
+ xunjieli@chromium.org

[eroman, 2017-05-01 19:42:26]

Screenshots showing how this looks are available at:

https://bugs.chromium.org/p/chromium/issues/detail?id=701033#c16

[xunjieli, 2017-05-02 17:10:02]

Looks great both on desktop and on mobile! I only have a few nits below.

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
File components/net_log/resources/net_export.css (right):

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
components/net_log/resources/net_export.css:8: margin: 1em;
Should we make the left-margin and right-margin auto?
On desktop, the view is at left corner of the screen. Maybe we can center it?

Screenshot:
https://drive.google.com/file/d/0Bw0C8-TgzJfsXzBaTFA2cDUwQnc/view?usp=sharing

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
File components/net_log/resources/net_export.html (right):

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
components/net_log/resources/net_export.html:133: <li>There be some cached
hostnames/URLs from sites visited prior
nit: There be -> There can be ?

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
File components/net_log/resources/net_export.js (right):

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
components/net_log/resources/net_export.js:36: var kIdReadMoreDiv =
'export-view-privacy-read-more'
Do these elements need an "export-view" prefix? The names are rather long. If we
can get rid of "export-view" prefix, it's easier to read and correlate the
values in the CSS file with those in the HTML file.
I noticed that the new elements that you added ("section-container" and
"outline-box") do not have this prefix, so i guess the prefix is not strictly
needed.

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
components/net_log/resources/net_export.js:69: * Sends NetLog data via email
from browser (mobile only)
nit: add a period at the end.

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
components/net_log/resources/net_export.js:169: renderUninitialized_:
function(info) {
Why do we need to pass |info| here and for renderInitial_? If it's not needed,
maybe we could eliminate the argument?

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
components/net_log/resources/net_export.js:216: * Get the textual label for a
capture mode from the HTML.
extra nitpicky: s/Get/Gets

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
components/net_log/resources/net_export.js:222: let captureMode =
info.captureMode;
nit: this local variable is used in exactly one place, could consider inlining
it?

[eroman, 2017-05-02 20:07:45]

The CQ bit was checked by eroman@chromium.org to run a CQ dry run

[eroman, 2017-05-02 20:07:59]

Thanks for the feedback!

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
File components/net_log/resources/net_export.css (right):

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
components/net_log/resources/net_export.css:8: margin: 1em;
On 2017/05/02 17:10:01, xunjieli wrote:
> Should we make the left-margin and right-margin auto?
> On desktop, the view is at left corner of the screen. Maybe we can center it?
> 
> Screenshot:
> https://drive.google.com/file/d/0Bw0C8-TgzJfsXzBaTFA2cDUwQnc/view?usp=sharing
> 

Done

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
File components/net_log/resources/net_export.html (right):

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
components/net_log/resources/net_export.html:133: <li>There be some cached
hostnames/URLs from sites visited prior
On 2017/05/02 17:10:01, xunjieli wrote:
> nit: There be -> There can be ?

Done.

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
File components/net_log/resources/net_export.js (right):

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
components/net_log/resources/net_export.js:36: var kIdReadMoreDiv =
'export-view-privacy-read-more'
On 2017/05/02 17:10:01, xunjieli wrote:
> Do these elements need an "export-view" prefix? The names are rather long. If
we
> can get rid of "export-view" prefix, it's easier to read and correlate the
> values in the CSS file with those in the HTML file.
> I noticed that the new elements that you added ("section-container" and
> "outline-box") do not have this prefix, so i guess the prefix is not strictly
> needed.

Done.

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
components/net_log/resources/net_export.js:69: * Sends NetLog data via email
from browser (mobile only)
On 2017/05/02 17:10:01, xunjieli wrote:
> nit: add a period at the end.

Done.

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
components/net_log/resources/net_export.js:169: renderUninitialized_:
function(info) {
On 2017/05/02 17:10:01, xunjieli wrote:
> Why do we need to pass |info| here and for renderInitial_? If it's not needed,
> maybe we could eliminate the argument?

Done.

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
components/net_log/resources/net_export.js:216: * Get the textual label for a
capture mode from the HTML.
On 2017/05/02 17:10:01, xunjieli wrote:
> extra nitpicky: s/Get/Gets

Done.

https://codereview.chromium.org/2850253002/diff/1/components/net_log/resource...
components/net_log/resources/net_export.js:222: let captureMode =
info.captureMode;
On 2017/05/02 17:10:01, xunjieli wrote:
> nit: this local variable is used in exactly one place, could consider inlining
> it?

Done.

[commit-bot: I haz the power, 2017-05-02 20:08:03]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eroman, 2017-05-02 20:11:03]

The CQ bit was checked by eroman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-02 20:11:21]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-02 21:05:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-02 21:05:40]

Dry run: This issue passed the CQ dry run.

[xunjieli, 2017-05-02 21:30:01]

lgtm!

[eroman, 2017-05-02 21:39:31]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2017-05-02 21:40:02]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-02 21:45:49]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493761171834040,
"parent_rev": "290bb6a21076dfeda8103206fe39a61ab9b5ed25", "commit_rev":
"912ec7d24049de83f175b212da8233edf78a30e2"}

[commit-bot: I haz the power, 2017-05-02 21:46:00]

Description was changed from

==========
Change chrome://net-export/ UI flow.

Instead of an all-in-one page displaying the current state and disabled
future states, splits the user journey among separate screens, each
with their own instructions.

BUG=701033
==========

to

==========
Change chrome://net-export/ UI flow.

Instead of an all-in-one page displaying the current state and disabled
future states, splits the user journey among separate screens, each
with their own instructions.

BUG=701033

Review-Url: https://codereview.chromium.org/2850253002
Cr-Commit-Position: refs/heads/master@{#468783}
Committed:
https://chromium.googlesource.com/chromium/src/+/912ec7d24049de83f175b212da82...
==========

[commit-bot: I haz the power, 2017-05-02 21:46:05]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/912ec7d24049de83f175b212da82...