[Signin] Update order of buttons in dialog for merging data

[Signin] Update order of buttons in dialog for merging data

The first one should always be the default one.

Updated switch case: http://screen/PTLfmU9H7h0.png

BUG=660418
Review-Url: https://codereview.chromium.org/2614973002
Cr-Commit-Position: refs/heads/master@{#442977}
Committed: https://chromium.googlesource.com/chromium/src/+/302d9ac35bbd6fa65f9364388c1fb381ae58060a

[bzanotti, 2017-01-05 13:58:35]

bzanotti@chromium.org changed reviewers:
+ gogerald@chromium.org, twellington@chromium.org

[bzanotti, 2017-01-05 13:58:36]

Please take a look.

[gogerald1, 2017-01-05 15:39:38]

https://codereview.chromium.org/2614973002/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/confirm_import_sync_data.xml (right):

https://codereview.chromium.org/2614973002/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/confirm_import_sync_data.xml:16:
android:id="@+id/sync_import_data_linear_layout"
nit: sync_import_data or sync_import_data_content? ..._linear_layout looks a
little strange

https://codereview.chromium.org/2614973002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/signin/ConfirmImportSyncDataDialog.java
(right):

https://codereview.chromium.org/2614973002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/signin/ConfirmImportSyncDataDialog.java:163:
// In the switching case, re-order the buttons so the checked one is at
nit: ...default checked one...

https://codereview.chromium.org/2614973002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/signin/ConfirmImportSyncDataDialog.java:171:
layout.addView(mConfirmImportOption);
Could you just remove the mConfirmImportOption, then add it back (will be added
to the end by default).

nit: move this logic in to line 157?
nit: you can call getParent to find the parent view.

[bzanotti, 2017-01-05 17:39:32]

PTAL

https://codereview.chromium.org/2614973002/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/confirm_import_sync_data.xml (right):

https://codereview.chromium.org/2614973002/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/confirm_import_sync_data.xml:16:
android:id="@+id/sync_import_data_linear_layout"
On 2017/01/05 15:39:38, gogerald1 wrote:
> nit: sync_import_data or sync_import_data_content? ..._linear_layout looks a
> little strange

Done.

https://codereview.chromium.org/2614973002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/signin/ConfirmImportSyncDataDialog.java
(right):

https://codereview.chromium.org/2614973002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/signin/ConfirmImportSyncDataDialog.java:163:
// In the switching case, re-order the buttons so the checked one is at
On 2017/01/05 15:39:38, gogerald1 wrote:
> nit: ...default checked one...

Done.

https://codereview.chromium.org/2614973002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/signin/ConfirmImportSyncDataDialog.java:171:
layout.addView(mConfirmImportOption);
On 2017/01/05 15:39:38, gogerald1 wrote:
> Could you just remove the mConfirmImportOption, then add it back (will be
added
> to the end by default).
> 
> nit: move this logic in to line 157?
> nit: you can call getParent to find the parent view.

Done.

I can't move it to line 157 as it would only be executed for non-managed
accounts.

I find it more readable to access the parent view via its id. I'll keep it this
way if it's fine with you.

[gogerald1, 2017-01-05 17:46:21]

lgtm

[Theresa, 2017-01-05 17:58:13]

lgtm

[bzanotti, 2017-01-05 18:02:30]

bzanotti@chromium.org changed reviewers:
+ nyquist@chromium.org

[bzanotti, 2017-01-05 18:02:30]

nyquist@: Please do an OWNERS review for the string change.

[bzanotti, 2017-01-11 17:33:38]

Ping.

[nyquist, 2017-01-11 18:43:51]

lgtm

[bzanotti, 2017-01-11 18:51:47]

The CQ bit was checked by bzanotti@chromium.org

[commit-bot: I haz the power, 2017-01-11 18:52:14]

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-01-11 19:27:19]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484160707278240,
"parent_rev": "7fec81ccbc6ccde77d9991ee7d731cc6305650cb", "commit_rev":
"302d9ac35bbd6fa65f9364388c1fb381ae58060a"}

[commit-bot: I haz the power, 2017-01-11 19:27:47]

Description was changed from

==========
[Signin] Update order of buttons in dialog for merging data

The first one should always be the default one.

Updated switch case: http://screen/PTLfmU9H7h0.png

BUG=660418
==========

to

==========
[Signin] Update order of buttons in dialog for merging data

The first one should always be the default one.

Updated switch case: http://screen/PTLfmU9H7h0.png

BUG=660418

Review-Url: https://codereview.chromium.org/2614973002
Cr-Commit-Position: refs/heads/master@{#442977}
Committed:
https://chromium.googlesource.com/chromium/src/+/302d9ac35bbd6fa65f9364388c1f...
==========

[commit-bot: I haz the power, 2017-01-11 19:27:50]

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