MD Settings: Stop using prefs to populate import data dialog.

MD Settings: Stop using prefs to populate import data dialog.

BUG=695611
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2717013002
Cr-Commit-Position: refs/heads/master@{#453332}
Committed: https://chromium.googlesource.com/chromium/src/+/5a915bf6d281b1142802a87491a1d17e811f3a8b

[dpapad, 2017-02-25 02:51:57]

Description was changed from

==========
MD Settings: Stop using prefs to populate import data dialog.

BUG=695611
==========

to

==========
MD Settings: Stop using prefs to populate import data dialog.

BUG=695611
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2017-02-25 02:57:32]

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

[commit-bot: I haz the power, 2017-02-25 02:57:43]

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-02-25 03:54:05]

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

[commit-bot: I haz the power, 2017-02-25 03:54:06]

Dry run: This issue passed the CQ dry run.

[dpapad, 2017-02-27 18:19:46]

dpapad@chromium.org changed reviewers:
+ dbeam@chromium.org

[Dan Beam, 2017-02-27 18:39:24]

lgtm w/nits

https://codereview.chromium.org/2717013002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/people_page/import_data_dialog.html
(right):

https://codereview.chromium.org/2717013002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/people_page/import_data_dialog.html:37:
min-height: var(--settings-row-min-height);
just some ideas/options:

can this style be re-used between .row and .description?

maybe

.row {
  min-height: var(--settings-row-min-height);
}

.description {
  align-items: center;
  display: flex;
}

and make the descriptions class="row description"?

also note: iron-flex-classes lets you do align-items: center; or display: flex
just via class="layout horizontal center" i think (don't really know whether
we're encouraging use of iron-flex-classes right now, though...)

https://codereview.chromium.org/2717013002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/people_page/import_data_dialog.js
(right):

https://codereview.chromium.org/2717013002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/people_page/import_data_dialog.js:84:
!(this.$.history.checked && this.selected_.history) &&
arguable nit: we should reverse these conditionals, as we shouldn't be querying
the DOM's checked-ness if it's invalid (i.e. hidden but checked)

ex:

  !(this.selected_.history && this.$.history.checked) &&

[dpapad, 2017-02-27 19:55:00]

https://codereview.chromium.org/2717013002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/people_page/import_data_dialog.html
(right):

https://codereview.chromium.org/2717013002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/people_page/import_data_dialog.html:37:
min-height: var(--settings-row-min-height);
On 2017/02/27 at 18:39:23, Dan Beam wrote:
> just some ideas/options:
> 
> can this style be re-used between .row and .description?
> 
> maybe
> 
> .row {
>   min-height: var(--settings-row-min-height);
> }
> 
> .description {
>   align-items: center;
>   display: flex;
> }
> 
> and make the descriptions class="row description"?
Done.

> 
> also note: iron-flex-classes lets you do align-items: center; or display: flex
just via class="layout horizontal center" i think (don't really know whether
we're encouraging use of iron-flex-classes right now, though...)
Ack.

https://codereview.chromium.org/2717013002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/people_page/import_data_dialog.js
(right):

https://codereview.chromium.org/2717013002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/people_page/import_data_dialog.js:84:
!(this.$.history.checked && this.selected_.history) &&
On 2017/02/27 at 18:39:24, Dan Beam wrote:
> arguable nit: we should reverse these conditionals, as we shouldn't be
querying the DOM's checked-ness if it's invalid (i.e. hidden but checked)
> 
> ex:
> 
>   !(this.selected_.history && this.$.history.checked) &&

Done.

[dpapad, 2017-02-27 19:57:21]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2017-02-27 19:57:22]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/2717013002/#ps20001
(title: "Nits.")

[commit-bot: I haz the power, 2017-02-27 19:57:49]

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-02-27 21:05:27]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488225441748950,
"parent_rev": "079887531e294438cef7d5dc986626b45768763f", "commit_rev":
"5a915bf6d281b1142802a87491a1d17e811f3a8b"}

[commit-bot: I haz the power, 2017-02-27 21:06:04]

Description was changed from

==========
MD Settings: Stop using prefs to populate import data dialog.

BUG=695611
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Stop using prefs to populate import data dialog.

BUG=695611
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2717013002
Cr-Commit-Position: refs/heads/master@{#453332}
Committed:
https://chromium.googlesource.com/chromium/src/+/5a915bf6d281b1142802a87491a1...
==========

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

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/5a915bf6d281b1142802a87491a1...

[Dan Beam, 2017-03-01 02:07:51]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2727513002/ by dbeam@chromium.org.

The reason for reverting is: We want to continue respecting policy and master
prefs in the importData dialog..