Enhanced Sync Confirmation modal

Enhanced Sync Confirmation modal

This is replacing https://codereview.chromium.org/1806353002/ which got lgtm but was abandoned due to design changes.

This patch includes no C++ logic changes and instead of the sync confirmation screen, it adds a link to the "activity controls" in the Sync setup page.

BUG=595349
Committed: https://crrev.com/11a598fe8a0f2ae63b0e18dcc48c3d0d6c6b6738
Cr-Commit-Position: refs/heads/master@{#387322}

[Moe, 2016-04-08 19:37:34]

Description was changed from

==========
Enhanced Sync Confirmation modal
==========

to

==========
Enhanced Sync Confirmation modal

This is replacing https://codereview.chromium.org/1806353002/ which got lgtm but
was abandoned due to design changes.

This patch includes no C++ logic changes and instead of the sync confirmation
screen, it adds a link to the "activity controls" in the Sync setup page.


BUG=595349
==========

[Moe, 2016-04-08 19:37:34]

mahmadi@chromium.org changed reviewers:
+ dbeam@chromium.org, estade@chromium.org, rkaplow@chromium.org,
rogerta@chromium.org, sky@chromium.org

[Moe, 2016-04-08 19:38:04]

Hi, Please review this patch that replaces
https://codereview.chromium.org/1806353002/

[rkaplow, 2016-04-08 20:43:21]

lgtm

actions lgtm

[sky, 2016-04-08 22:01:26]

You've added 5 reviewers. Please indicate who you expect to review what.

[Moe, 2016-04-11 13:08:11]

On 2016/04/08 22:01:26, sky wrote:
> You've added 5 reviewers. Please indicate who you expect to review what.

Hi, My bad. Please review the following:

c/b/resources dbeam@
c/b/ui/sync rogerta@
c/b/ui/views sky@ 
c/b/ui/webui/options dbeam@
c/b/ui/webui/signin rogerta@

+estade@

please find the mocks here:
https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Desktop%20S...

PS. dbeam@, I will send a follow up CL once this lands to make changes to the
chrome://md-settings\

[sky, 2016-04-11 15:33:05]

c/b/ui/views LGTM

[Roger Tawa OOO till Jul 10th, 2016-04-11 15:43:05]

c/b/ui/sync lgtm

[Dan Beam, 2016-04-14 04:05:56]

lgtm

https://codereview.chromium.org/1868273002/diff/1/chrome/browser/resources/op...
File chrome/browser/resources/options/sync_setup_overlay.html (right):

https://codereview.chromium.org/1868273002/diff/1/chrome/browser/resources/op...
chrome/browser/resources/options/sync_setup_overlay.html:199:
i18n-values=".innerHTML:personalizeGoogleServicesMessage">
nit: why are you wrapping this?

https://codereview.chromium.org/1868273002/diff/1/chrome/browser/resources/sy...
File chrome/browser/resources/sync_confirmation/sync_confirmation.html (right):

https://codereview.chromium.org/1868273002/diff/1/chrome/browser/resources/sy...
chrome/browser/resources/sync_confirmation/sync_confirmation.html:9: <!-- see
https://www.polymer-project.org/1.0/docs/devguide/styling.html#custom-style -->
i don't think you need this note

https://codereview.chromium.org/1868273002/diff/1/chrome/browser/resources/sy...
chrome/browser/resources/sync_confirmation/sync_confirmation.html:52:
i18n-content="syncConfirmationChromeSyncTitle"></div>
nit:

  <div class="title">$i18n{syncConfirmationChromeSyncTitle}</div>

will flicker less and cause less wrapping

[Moe, 2016-04-14 14:26:53]

Thanks everyone!

https://codereview.chromium.org/1868273002/diff/1/chrome/browser/resources/op...
File chrome/browser/resources/options/sync_setup_overlay.html (right):

https://codereview.chromium.org/1868273002/diff/1/chrome/browser/resources/op...
chrome/browser/resources/options/sync_setup_overlay.html:199:
i18n-values=".innerHTML:personalizeGoogleServicesMessage">
On 2016/04/14 04:05:56, Dan Beam wrote:
> nit: why are you wrapping this?

My bad. The div initially had a class name. I later removed it but forgot to
unwrap it.

https://codereview.chromium.org/1868273002/diff/1/chrome/browser/resources/sy...
File chrome/browser/resources/sync_confirmation/sync_confirmation.html (right):

https://codereview.chromium.org/1868273002/diff/1/chrome/browser/resources/sy...
chrome/browser/resources/sync_confirmation/sync_confirmation.html:9: <!-- see
https://www.polymer-project.org/1.0/docs/devguide/styling.html#custom-style -->
On 2016/04/14 04:05:56, Dan Beam wrote:
> i don't think you need this note

Done.

https://codereview.chromium.org/1868273002/diff/1/chrome/browser/resources/sy...
chrome/browser/resources/sync_confirmation/sync_confirmation.html:52:
i18n-content="syncConfirmationChromeSyncTitle"></div>
On 2016/04/14 04:05:56, Dan Beam wrote:
> nit:
> 
>   <div class="title">$i18n{syncConfirmationChromeSyncTitle}</div>
> 
> will flicker less and cause less wrapping

Cool! Done.

[Moe, 2016-04-14 15:27:49]

The CQ bit was checked by mahmadi@chromium.org

[Moe, 2016-04-14 15:27:49]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org,
rogerta@chromium.org, rkaplow@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1868273002/#ps20001
(title: "Addressed Dan's comments + rebase")

[commit-bot: I haz the power, 2016-04-14 15:28:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1868273002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1868273002/20001

[commit-bot: I haz the power, 2016-04-14 15:34:36]

Description was changed from

==========
Enhanced Sync Confirmation modal

This is replacing https://codereview.chromium.org/1806353002/ which got lgtm but
was abandoned due to design changes.

This patch includes no C++ logic changes and instead of the sync confirmation
screen, it adds a link to the "activity controls" in the Sync setup page.


BUG=595349
==========

to

==========
Enhanced Sync Confirmation modal

This is replacing https://codereview.chromium.org/1806353002/ which got lgtm but
was abandoned due to design changes.

This patch includes no C++ logic changes and instead of the sync confirmation
screen, it adds a link to the "activity controls" in the Sync setup page.


BUG=595349
==========

[commit-bot: I haz the power, 2016-04-14 15:34:38]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-14 15:36:01]

Description was changed from

==========
Enhanced Sync Confirmation modal

This is replacing https://codereview.chromium.org/1806353002/ which got lgtm but
was abandoned due to design changes.

This patch includes no C++ logic changes and instead of the sync confirmation
screen, it adds a link to the "activity controls" in the Sync setup page.


BUG=595349
==========

to

==========
Enhanced Sync Confirmation modal

This is replacing https://codereview.chromium.org/1806353002/ which got lgtm but
was abandoned due to design changes.

This patch includes no C++ logic changes and instead of the sync confirmation
screen, it adds a link to the "activity controls" in the Sync setup page.

BUG=595349

Committed: https://crrev.com/11a598fe8a0f2ae63b0e18dcc48c3d0d6c6b6738
Cr-Commit-Position: refs/heads/master@{#387322}
==========

[commit-bot: I haz the power, 2016-04-14 15:36:02]

Patchset 2 (id:??) landed as
https://crrev.com/11a598fe8a0f2ae63b0e18dcc48c3d0d6c6b6738
Cr-Commit-Position: refs/heads/master@{#387322}