[unified-consent] Revamp advanced sync setup during opt-in
- Add 'Cancel' and 'Confirm' buttons to sync account control
during the sync setup.
- Make cancel the default action when the user leaves the
sync setup without confirming.
- Remove snackbar from sync controls page.
Video:
https://drive.google.com/file/d/1tjn6Ey6ddbDff9xwOu9hP2-bJv-3b77U/view?usp=sharing
Bug: 919745
Change-Id: Ibf83e00cd4a7d7e3aab5eaf55e6bd2cdc17309cd
Reviewed-on: https://chromium-review.googlesource.com/c/1422000
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Thomas Tangl <tangltom@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#626355}(cherry picked from commit 1503157f8de7ecae2cc25d366e393e972e020057)
Reviewed-on: https://chromium-review.googlesource.com/c/1449678
Reviewed-by: Thomas Tangl <tangltom@chromium.org>
Cr-Commit-Position: refs/branch-heads/3683@{#115}
Cr-Branched-From: e51029943e0a38dd794b73caaf6373d5496ae783-refs/heads/master@{#625896}diff --git a/chrome/browser/resources/settings/people_page/people_page.html b/chrome/browser/resources/settings/people_page/people_page.html
index e7e91eef..08e32c43 100644
--- a/chrome/browser/resources/settings/people_page/people_page.html
+++ b/chrome/browser/resources/settings/people_page/people_page.html
@@ -331,8 +331,7 @@
<template is="dom-if" route-path="/syncSetup/advanced" no-search>
<settings-subpage page-title="$i18n{syncAdvancedPageTitle}"
learn-more-url="$i18n{syncAndGoogleServicesLearnMoreURL}">
- <settings-sync-controls sync-status="[[syncStatus]]"
- on-sync-setup-cancel="cancelSyncSetup_">
+ <settings-sync-controls sync-status="[[syncStatus]]">
</settings-sync-controls>
</settings-subpage>
</template>
diff --git a/chrome/browser/resources/settings/people_page/people_page.js b/chrome/browser/resources/settings/people_page/people_page.js
index 33f6656..bdff0413 100644
--- a/chrome/browser/resources/settings/people_page/people_page.js
+++ b/chrome/browser/resources/settings/people_page/people_page.js
@@ -582,9 +582,4 @@
}
return this.i18n('lockScreenTitleLock');
},
-
- /** @private */
- cancelSyncSetup_: function() {
- this.$$('settings-sync-page').cancelSyncSetup();
- },
});
diff --git a/chrome/browser/resources/settings/people_page/sync_account_control.html b/chrome/browser/resources/settings/people_page/sync_account_control.html
index d22f950..51754fd 100644
--- a/chrome/browser/resources/settings/people_page/sync_account_control.html
+++ b/chrome/browser/resources/settings/people_page/sync_account_control.html
@@ -190,17 +190,27 @@
$i18n{peopleSignIn}
</paper-button>
<paper-button id="turn-off" class="secondary-button"
- hidden="[[!shouldShowTurnOffButton_(syncStatus.signedIn)]]"
+ hidden="[[!shouldShowTurnOffButton_(syncStatus.signedIn,
+ showSetupButtons_)]]"
on-click="onTurnOffButtonTap_"
disabled="[[syncStatus.setupInProgress]]">
$i18n{turnOffSync}
</paper-button>
<paper-button id="sync-error-button" class="action-button"
- hidden="[[!shouldShowErrorActionButton_(syncStatus)]]"
+ hidden="[[!shouldShowErrorActionButton_(syncStatus,
+ showSetupButtons_)]]"
on-click="onErrorButtonTap_"
disabled="[[syncStatus.setupInProgress]]">
[[syncStatus.statusActionText]]
</paper-button>
+ <div id="setup-buttons" hidden="[[!showSetupButtons_]]">
+ <paper-button class="secondary-button" on-click="onSetupCancel_">
+ $i18n{cancel}
+ </paper-button>
+ <paper-button class="action-button" on-click="onSetupConfirm_">
+ $i18n{confirm}
+ </paper-button>
+ </div>
</div>
<template is="dom-if" if="[[!syncStatus.signedIn]]" restamp>
<cr-action-menu id="menu" auto-reposition>
diff --git a/chrome/browser/resources/settings/people_page/sync_account_control.js b/chrome/browser/resources/settings/people_page/sync_account_control.js
index dfff900..cba1c3b 100644
--- a/chrome/browser/resources/settings/people_page/sync_account_control.js
+++ b/chrome/browser/resources/settings/people_page/sync_account_control.js
@@ -88,6 +88,13 @@
},
unifiedConsentEnabled: Boolean,
+
+ /** @private */
+ showSetupButtons_: {
+ type: Boolean,
+ computed: 'computeShowSetupButtons_(unifiedConsentEnabled,' +
+ 'hideButtons, syncStatus.setupInProgress)',
+ },
},
observers: [
@@ -282,7 +289,8 @@
* @private
*/
shouldShowTurnOffButton_: function() {
- return !this.hideButtons && !!this.syncStatus.signedIn;
+ return !this.hideButtons && !this.showSetupButtons_ &&
+ !!this.syncStatus.signedIn;
},
/**
@@ -296,8 +304,8 @@
// In a subpage the passphrase button is not required.
return false;
}
- return !this.hideButtons && !!this.syncStatus.signedIn &&
- !!this.syncStatus.hasError &&
+ return !this.hideButtons && !this.showSetupButtons_ &&
+ !!this.syncStatus.signedIn && !!this.syncStatus.hasError &&
this.syncStatus.statusAction != settings.StatusAction.NO_ACTION;
},
@@ -440,5 +448,24 @@
this.recordImpressionUserActions_();
}
}
- }
+ },
+
+ /**
+ * @return {boolean}
+ * @private
+ */
+ computeShowSetupButtons_: function() {
+ return !this.hideButtons && !!this.unifiedConsentEnabled &&
+ !!this.syncStatus.setupInProgress;
+ },
+
+ /** @private */
+ onSetupCancel_: function() {
+ this.fire('sync-setup-done', false);
+ },
+
+ /** @private */
+ onSetupConfirm_: function() {
+ this.fire('sync-setup-done', true);
+ },
});
diff --git a/chrome/browser/resources/settings/people_page/sync_controls.html b/chrome/browser/resources/settings/people_page/sync_controls.html
index 514db94..b809d8e 100644
--- a/chrome/browser/resources/settings/people_page/sync_controls.html
+++ b/chrome/browser/resources/settings/people_page/sync_controls.html
@@ -9,10 +9,6 @@
<link rel="import" href="../settings_shared_css.html">
<link rel="import" href="../settings_vars_css.html">
-<if expr="not chromeos">
-<link rel="import" href="chrome://resources/cr_elements/cr_toast/cr_toast.html">
-</if>
-
<dom-module id="settings-sync-controls">
<template>
<style include="settings-shared">
@@ -27,18 +23,6 @@
.list-item > div {
flex: 1;
}
-
-<if expr="not chromeos">
- #toast {
- left: 0;
- z-index: 1;
- }
-
- :host-context([dir='rtl']) #toast {
- left: auto;
- right: 0;
- }
-</if>
</style>
<div class="settings-box first">
@@ -182,16 +166,6 @@
</cr-toggle>
</div>
</div>
-
-<if expr="not chromeos">
- <cr-toast id="toast"
- open="[[shouldShowSyncSetupToast_(syncStatus.setupInProgress)]]">
- <div>$i18n{syncWillStart}</div>
- <paper-button on-click="onCancelSyncClick_">
- $i18n{cancelSync}
- </paper-button>
- </cr-toast>
-</if>
</template>
<script src="sync_controls.js"></script>
</dom-module>
diff --git a/chrome/browser/resources/settings/people_page/sync_controls.js b/chrome/browser/resources/settings/people_page/sync_controls.js
index 896ff12..5ba595a4 100644
--- a/chrome/browser/resources/settings/people_page/sync_controls.js
+++ b/chrome/browser/resources/settings/people_page/sync_controls.js
@@ -160,20 +160,6 @@
return syncAllDataTypes || !autofillSynced;
},
- /**
- * @private
- * @return {boolean}
- */
- shouldShowSyncSetupToast_: function() {
- return settings.getCurrentRoute() == settings.routes.SYNC_ADVANCED &&
- this.syncStatus.setupInProgress;
- },
-
- /** @private */
- onCancelSyncClick_: function() {
- this.fire('sync-setup-cancel');
- },
-
/** @private */
syncStatusChanged_: function(syncStatus) {
// When the sync controls are embedded, the parent has to take care of
diff --git a/chrome/browser/resources/settings/people_page/sync_page.html b/chrome/browser/resources/settings/people_page/sync_page.html
index a2530a21..601ecbb 100644
--- a/chrome/browser/resources/settings/people_page/sync_page.html
+++ b/chrome/browser/resources/settings/people_page/sync_page.html
@@ -105,7 +105,8 @@
promo-label-with-account=
"$i18n{peopleSignInPromptSecondaryWithAccount}"
promo-label-with-no-account=
- "$i18n{peopleSignInPromptSecondaryWithNoAccount}">
+ "$i18n{peopleSignInPromptSecondaryWithNoAccount}"
+ on-sync-setup-done="onSyncSetupDone_">
</settings-sync-account-control>
</template>
</if>
@@ -332,12 +333,14 @@
</template>
<if expr="not chromeos">
- <cr-toast id="toast" open="[[syncStatus.setupInProgress]]">
- <div>$i18n{syncWillStart}</div>
- <paper-button on-click="cancelSyncSetup">
- $i18n{cancelSync}
- </paper-button>
- </cr-toast>
+ <template is="dom-if" if="[[!unifiedConsentEnabled]]">
+ <cr-toast id="toast" open="[[syncStatus.setupInProgress]]">
+ <div>$i18n{syncWillStart}</div>
+ <paper-button on-click="onSyncSetupCancel_">
+ $i18n{cancelSync}
+ </paper-button>
+ </cr-toast>
+ </template>
</if>
</template>
<script src="sync_page.js"></script>
diff --git a/chrome/browser/resources/settings/people_page/sync_page.js b/chrome/browser/resources/settings/people_page/sync_page.js
index fa8ba3af..e7377df 100644
--- a/chrome/browser/resources/settings/people_page/sync_page.js
+++ b/chrome/browser/resources/settings/people_page/sync_page.js
@@ -137,7 +137,10 @@
diceEnabled: Boolean,
// </if>
- unifiedConsentEnabled: Boolean,
+ unifiedConsentEnabled: {
+ type: Boolean,
+ observer: 'initializeDidAbort_',
+ },
},
/** @private {?settings.SyncBrowserProxy} */
@@ -148,8 +151,8 @@
* if the user has closed the tab with the sync settings. This property is
* non-null if the user is currently navigated on the sync settings route.
*
- * TODO(scottchen): We had to change from unload to beforeunload due to
- * crbug.com/501292. Change back to unload once it's fixed.
+ * TODO(crbug.com/862983): When unified consent is rolled out to 100% this
+ * should be removed.
*
* @private {?Function}
*/
@@ -163,7 +166,8 @@
collapsibleSectionsInitialized_: false,
/**
- * Whether the user decided to abort sync.
+ * Whether the user decided to abort sync. When unified consent is enabled,
+ * this is initialized to true.
* @private {boolean}
*/
didAbort_: false,
@@ -184,14 +188,6 @@
this.onNavigateToPage_();
}
},
- /**
- * Can be called from subpages to notify this page (=main sync page) that sync
- * setup was cancelled.
- */
- cancelSyncSetup: function() {
- this.didAbort_ = true;
- settings.navigateTo(settings.routes.BASIC);
- },
/** @override */
detached: function() {
@@ -281,7 +277,7 @@
this.pageStatus_ = settings.PageStatus.CONFIGURE;
this.browserProxy_.didNavigateAwayFromSyncPage(this.didAbort_);
- this.didAbort_ = false;
+ this.initializeDidAbort_();
window.removeEventListener('beforeunload', this.beforeunloadCallback_);
this.beforeunloadCallback_ = null;
@@ -543,6 +539,29 @@
return loadTimeData.getBoolean('driveSuggestAvailable') &&
!this.unifiedConsentEnabled;
},
+
+ /** @private */
+ onSyncSetupCancel_: function() {
+ this.didAbort_ = true;
+ settings.navigateTo(settings.routes.BASIC);
+ },
+
+ /** @private */
+ initializeDidAbort_: function() {
+ this.didAbort_ = !!this.unifiedConsentEnabled;
+ },
+
+ /**
+ * @param {!CustomEvent<boolean>} e The event passed from
+ * settings-sync-account-control.
+ * @private
+ */
+ onSyncSetupDone_: function(e) {
+ if (e.detail) {
+ this.didAbort_ = false;
+ }
+ settings.navigateTo(settings.routes.BASIC);
+ },
});
})();
diff --git a/chrome/test/data/webui/settings/cr_settings_browsertest.js b/chrome/test/data/webui/settings/cr_settings_browsertest.js
index 3321322..47a93936 100644
--- a/chrome/test/data/webui/settings/cr_settings_browsertest.js
+++ b/chrome/test/data/webui/settings/cr_settings_browsertest.js
@@ -677,6 +677,7 @@
/** @override */
extraLibraries: CrSettingsBrowserTest.prototype.extraLibraries.concat([
'../test_browser_proxy.js',
+ 'sync_test_util.js',
'test_sync_browser_proxy.js',
'test_util.js',
'people_page_sync_page_test.js',
diff --git a/chrome/test/data/webui/settings/people_page_sync_controls_test.js b/chrome/test/data/webui/settings/people_page_sync_controls_test.js
index e5b902e..64a4a3d3 100644
--- a/chrome/test/data/webui/settings/people_page_sync_controls_test.js
+++ b/chrome/test/data/webui/settings/people_page_sync_controls_test.js
@@ -105,16 +105,6 @@
}
return browserProxy.whenCalled('setSyncDatatypes').then(verifyPrefs);
});
-
- if (!cr.isChromeOS) {
- test('ToastNotShownWhenEmbedded', function() {
- syncControls.syncStatus = {setupInProgress: false};
- assertFalse(syncControls.$.toast.open);
-
- syncControls.syncStatus = {setupInProgress: true};
- assertFalse(syncControls.$.toast.open);
- });
- }
});
suite('SyncControlsSubpageTest', function() {
@@ -142,18 +132,6 @@
syncControls.remove();
});
- if (!cr.isChromeOS) {
- test('ToastShownForSyncSetup', function() {
- syncControls.syncStatus = {setupInProgress: false, signedIn: true};
- assertFalse(syncControls.$.toast.open);
-
- syncControls.syncStatus = {setupInProgress: true, signedIn: true};
- assertTrue(syncControls.$.toast.open);
-
- syncControls.$.toast.querySelector('paper-button').click();
- });
- }
-
test('SignedOut', function() {
syncControls
.syncStatus = {disabled: false, hasError: false, signedIn: false};
diff --git a/chrome/test/data/webui/settings/people_page_sync_page_test.js b/chrome/test/data/webui/settings/people_page_sync_page_test.js
index ac2c34f..b7b2e1e 100644
--- a/chrome/test/data/webui/settings/people_page_sync_page_test.js
+++ b/chrome/test/data/webui/settings/people_page_sync_page_test.js
@@ -490,14 +490,92 @@
});
if (!cr.isChromeOS) {
- test('FirstTimeSetupNotification', function() {
- assertTrue(!!syncPage.$.toast);
- assertFalse(syncPage.$.toast.open);
+ test('SyncSetupCancel_UnifiedConsentDisabled', function() {
+ syncPage.unifiedConsentEnabled = false;
+ Polymer.dom.flush();
+
+ const toast = syncPage.$$('cr-toast');
+ assertTrue(!!toast);
+ assertFalse(toast.open);
syncPage.syncStatus = {setupInProgress: true};
Polymer.dom.flush();
- assertTrue(syncPage.$.toast.open);
+ assertTrue(toast.open);
- syncPage.$.toast.querySelector('paper-button').click();
+ toast.querySelector('paper-button').click();
+
+ return browserProxy.whenCalled('didNavigateAwayFromSyncPage')
+ .then(abort => {
+ assertTrue(abort);
+ });
+ });
+
+ test('SyncSetupLeavePage UnifiedConsentDisabled', function() {
+ syncPage.unifiedConsentEnabled = false;
+ Polymer.dom.flush();
+
+ settings.navigateTo(settings.routes.BASIC);
+
+ return browserProxy.whenCalled('didNavigateAwayFromSyncPage')
+ .then(abort => {
+ assertFalse(abort);
+ });
+ });
+
+ test('SyncSetupCancel UnifiedConsentEnabled', function() {
+ syncPage.diceEnabled = true;
+ syncPage.unifiedConsentEnabled = true;
+ syncPage.syncStatus = {
+ signinAllowed: true,
+ syncSystemEnabled: true,
+ setupInProgress: true,
+ signedIn: true
+ };
+ Polymer.dom.flush();
+ sync_test_util.simulateStoredAccounts([{email: 'foo@foo.com'}]);
+
+ const cancelButton =
+ syncPage.$$('settings-sync-account-control')
+ .shadowRoot.querySelector('#setup-buttons .secondary-button');
+
+ assertTrue(!!cancelButton);
+ cancelButton.click();
+
+ return browserProxy.whenCalled('didNavigateAwayFromSyncPage')
+ .then(abort => {
+ assertTrue(abort);
+ });
+ });
+
+ test('SyncSetupConfirm UnifiedConsentEnabled', function() {
+ syncPage.diceEnabled = true;
+ syncPage.unifiedConsentEnabled = true;
+ syncPage.syncStatus = {
+ signinAllowed: true,
+ syncSystemEnabled: true,
+ setupInProgress: true,
+ signedIn: true
+ };
+ Polymer.dom.flush();
+ sync_test_util.simulateStoredAccounts([{email: 'foo@foo.com'}]);
+
+ const confirmButton =
+ syncPage.$$('settings-sync-account-control')
+ .shadowRoot.querySelector('#setup-buttons .action-button');
+
+ assertTrue(!!confirmButton);
+ confirmButton.click();
+
+ return browserProxy.whenCalled('didNavigateAwayFromSyncPage')
+ .then(abort => {
+ assertFalse(abort);
+ });
+ });
+
+ test('SyncSetupLeavePage UnifiedConsentEnabled', function() {
+ syncPage.unifiedConsentEnabled = true;
+ Polymer.dom.flush();
+
+ settings.navigateTo(settings.routes.BASIC);
return browserProxy.whenCalled('didNavigateAwayFromSyncPage')
.then(abort => {
diff --git a/chrome/test/data/webui/settings/people_page_test.js b/chrome/test/data/webui/settings/people_page_test.js
index bb038a4..573c02a 100644
--- a/chrome/test/data/webui/settings/people_page_test.js
+++ b/chrome/test/data/webui/settings/people_page_test.js
@@ -580,23 +580,5 @@
assertEquals(settings.getCurrentRoute(), settings.routes.SYNC);
});
-
- if (!cr.isChromeOS) {
- test('CancelSyncSetupOnSyncControlsPage', function() {
- settings.navigateTo(settings.routes.SYNC);
- settings.navigateTo(settings.routes.SYNC_ADVANCED);
- peoplePage.syncStatus = {setupInProgress: true};
- Polymer.dom.flush();
-
- peoplePage.$$('settings-sync-controls')
- .$.toast.querySelector('paper-button')
- .click();
-
- return browserProxy.whenCalled('didNavigateAwayFromSyncPage')
- .then(abort => {
- assertTrue(abort);
- });
- });
- }
});
});
diff --git a/chrome/test/data/webui/settings/sync_account_control_test.js b/chrome/test/data/webui/settings/sync_account_control_test.js
index 7fa0c336..4ac7b93 100644
--- a/chrome/test/data/webui/settings/sync_account_control_test.js
+++ b/chrome/test/data/webui/settings/sync_account_control_test.js
@@ -345,14 +345,17 @@
};
Polymer.dom.flush();
const userInfo = testElement.$$('#user-info');
+ const setupButtons = testElement.$$('#setup-buttons');
assertTrue(userInfo.textContent.includes('barName'));
assertFalse(userInfo.textContent.includes('Setup in progress...'));
+ assertVisible(setupButtons, false);
testElement.unifiedConsentEnabled = true;
assertTrue(userInfo.textContent.includes('barName'));
assertTrue(userInfo.textContent.includes('Setup in progress...'));
+ assertVisible(setupButtons, true);
});
test('embedded in another page', function() {
