Commit 8d4d0ff0 authored by Nina Satragno's avatar Nina Satragno Committed by Commit Bot

Revert "[SyncSetup]: Change Sync everything to a radio behaviour."

This reverts commit ed985006.

Reason for revert: I suspect this is the cause of several flaky
tests on mac, see crbug.com/1043665

Original change's description:
> [SyncSetup]: Change Sync everything to a radio behaviour.
> 
> As part of friendly settings, this CL implements changing |sync
> everything| from check box to radio button. The motivation behind this
> work is to clarify to users the difference between |Sync everything|
> checked and |Sync everything| unchecked but all single data types
> checked.
> 
> Bug: 1043122
> Change-Id: I579b8173a7b32f1416a3b1eda999196d177a2b5c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2007730
> Commit-Queue: Monica Basta <msalama@chromium.org>
> Reviewed-by: Dan Beam <dbeam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#733286}

TBR=dbeam@chromium.org,msalama@chromium.org

Change-Id: Ibe071bf447abb103d3ae801069cf1f27a9123c67
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1043122
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2011168Reviewed-by: default avatarNina Satragno <nsatragno@chromium.org>
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733420}
parent 0f9c87a6
...@@ -4402,21 +4402,6 @@ ...@@ -4402,21 +4402,6 @@
</message> </message>
<message name="IDS_SETTINGS_SYNC_EVERYTHING_CHECKBOX_LABEL" desc="Label for the checkbox which causes all properties to be synced."> <message name="IDS_SETTINGS_SYNC_EVERYTHING_CHECKBOX_LABEL" desc="Label for the checkbox which causes all properties to be synced.">
Sync everything Sync everything
</message>
<message name="IDS_SETTINGS_SYNC_EVERYTHING" desc="Label for the radio button which causes all properties to be synced.">
Sync everything
</message>
<message name="IDS_SETTINGS_SYNC_EVERYTHING_HINT" desc="Hint for the radio button which causes all properties to be synced.">
Sync all data types including ones that might be added in the future
</message>
<message name="IDS_SETTINGS_CUSTOMIZE_SYNC" desc="Label for the radio button to customize sync data types.">
Customize sync
</message>
<message name="IDS_SETTINGS_CUSTOMIZE_SYNC_HINT" desc="Hint or the radio button to customize sync data types.">
Sync only selected data types
</message>
<message name="IDS_SETTINGS_SYNC_DATA" desc="Title for sync data types list.">
Sync data
</message> </message>
<message name="IDS_SETTINGS_MANAGE_GOOGLE_ACCOUNT" desc="Label for the button that takes the user to the Google Account website."> <message name="IDS_SETTINGS_MANAGE_GOOGLE_ACCOUNT" desc="Label for the button that takes the user to the Google Account website.">
Manage your Google Account Manage your Google Account
......
...@@ -22,51 +22,17 @@ ...@@ -22,51 +22,17 @@
.list-item > div { .list-item > div {
flex: 1; flex: 1;
} }
cr-radio-button[name='sync-everything'] > .secondary {
border-bottom: var(--cr-separator-line);
padding-bottom: var(--cr-section-vertical-padding);
}
cr-radio-button[name='customize-sync'] {
padding-top: var(--cr-section-vertical-padding);
}
</style> </style>
<div class="settings-box first">
<template is="dom-if" if="[[syncSetupFriendlySettings_]]"> <div id="syncEverythingCheckboxLabel" class="start">
<div id="sync-data-radio" class="settings-box first"> $i18n{syncEverythingCheckboxLabel}
<cr-radio-group
selected="[[selectedSyncDataRadio_(syncPrefs)]]"
on-selected-changed="onSyncDataRadioSelectionChanged_">
<cr-radio-button name="sync-everything">
$i18n{syncEverythingLabel}
<div class="secondary">$i18n{syncEverythingHint}</div>
</cr-radio-button>
<cr-radio-button name="customize-sync">
$i18n{customizeSyncLabel}
<div class="secondary">$i18n{customizeSyncHint}</div>
</cr-radio-button>
</cr-radio-group>
</div>
<div class="settings-box first">
<h2 class="cr-title-text start">$i18n{syncData}</h2>
</div> </div>
</template> <cr-toggle id="syncAllDataTypesControl"
checked="{{syncPrefs.syncAllDataTypes}}"
on-change="onSyncAllDataTypesChanged_"
<template is="dom-if" if="[[!syncSetupFriendlySettings_]]"> aria-labelledby="syncEverythingCheckboxLabel">
<div class="settings-box first"> </cr-toggle>
<div id="syncEverythingCheckboxLabel" class="start"> </div>
$i18n{syncEverythingCheckboxLabel}
</div>
<cr-toggle id="syncAllDataTypesControl"
checked="{{syncPrefs.syncAllDataTypes}}"
on-change="onSyncAllDataTypesChanged_"
aria-labelledby="syncEverythingCheckboxLabel">
</cr-toggle>
</div>
</template>
<div class="list-frame" id="sync-data-types"> <div class="list-frame" id="sync-data-types">
<div class="list-item" hidden="[[!syncPrefs.appsRegistered]]"> <div class="list-item" hidden="[[!syncPrefs.appsRegistered]]">
......
...@@ -22,16 +22,6 @@ const SyncPrefsIndividualDataTypes = [ ...@@ -22,16 +22,6 @@ const SyncPrefsIndividualDataTypes = [
'paymentsIntegrationEnabled', 'paymentsIntegrationEnabled',
]; ];
/**
* Names of the radio buttons which allow the user to choose their data sync
* mechanism.
* @enum {string}
*/
const RadioButtonNames = {
SYNC_EVERYTHING: 'sync-everything',
CUSTOMIZE_SYNC: 'customize-sync',
};
/** /**
* @fileoverview * @fileoverview
* 'settings-sync-controls' contains all sync data type controls. * 'settings-sync-controls' contains all sync data type controls.
...@@ -64,18 +54,6 @@ Polymer({ ...@@ -64,18 +54,6 @@ Polymer({
type: Object, type: Object,
observer: 'syncStatusChanged_', observer: 'syncStatusChanged_',
}, },
/**
* If sync page friendly settings is enabled.
* @private
*/
syncSetupFriendlySettings_: {
type: Boolean,
value: function() {
return loadTimeData.valueExists('syncSetupFriendlySettings') &&
loadTimeData.getBoolean('syncSetupFriendlySettings');
}
},
}, },
/** @private {?settings.SyncBrowserProxy} */ /** @private {?settings.SyncBrowserProxy} */
...@@ -117,42 +95,6 @@ Polymer({ ...@@ -117,42 +95,6 @@ Polymer({
} }
}, },
/**
* Computed binding returning the selected sync data radio button.
* @private
*/
selectedSyncDataRadio_: function() {
return this.syncPrefs.syncAllDataTypes ? RadioButtonNames.SYNC_EVERYTHING :
RadioButtonNames.CUSTOMIZE_SYNC;
},
/**
* Called when the sync data radio button selection changes.
* @param {!Event} event
* @private
*/
onSyncDataRadioSelectionChanged_: function(event) {
if (event.detail.value === RadioButtonNames.SYNC_EVERYTHING) {
this.set('syncPrefs.syncAllDataTypes', true);
// Cache the previously selected preference before checking every box.
this.cachedSyncPrefs_ = {};
for (const dataType of SyncPrefsIndividualDataTypes) {
// These are all booleans, so this shallow copy is sufficient.
this.cachedSyncPrefs_[dataType] = this.syncPrefs[dataType];
this.set(['syncPrefs', dataType], true);
}
} else {
this.set('syncPrefs.syncAllDataTypes', false);
if (this.cachedSyncPrefs_) {
// Restore the previously selected preference.
for (const dataType of SyncPrefsIndividualDataTypes) {
this.set(['syncPrefs', dataType], this.cachedSyncPrefs_[dataType]);
}
}
}
this.onSingleSyncDataTypeChanged_();
},
/** /**
* Handler for when the sync all data types checkbox is changed. * Handler for when the sync all data types checkbox is changed.
* @param {!Event} event * @param {!Event} event
......
...@@ -2006,11 +2006,6 @@ void AddPeopleStrings(content::WebUIDataSource* html_source, Profile* profile) { ...@@ -2006,11 +2006,6 @@ void AddPeopleStrings(content::WebUIDataSource* html_source, Profile* profile) {
{"syncTimeout", IDS_SETTINGS_SYNC_TIMEOUT}, {"syncTimeout", IDS_SETTINGS_SYNC_TIMEOUT},
{"syncEverythingCheckboxLabel", {"syncEverythingCheckboxLabel",
IDS_SETTINGS_SYNC_EVERYTHING_CHECKBOX_LABEL}, IDS_SETTINGS_SYNC_EVERYTHING_CHECKBOX_LABEL},
{"syncEverythingLabel", IDS_SETTINGS_SYNC_EVERYTHING},
{"syncEverythingHint", IDS_SETTINGS_SYNC_EVERYTHING_HINT},
{"customizeSyncLabel", IDS_SETTINGS_CUSTOMIZE_SYNC},
{"customizeSyncHint", IDS_SETTINGS_CUSTOMIZE_SYNC_HINT},
{"syncData", IDS_SETTINGS_SYNC_DATA},
{"manageGoogleAccount", IDS_SETTINGS_MANAGE_GOOGLE_ACCOUNT}, {"manageGoogleAccount", IDS_SETTINGS_MANAGE_GOOGLE_ACCOUNT},
{"appCheckboxLabel", IDS_SETTINGS_APPS_CHECKBOX_LABEL}, {"appCheckboxLabel", IDS_SETTINGS_APPS_CHECKBOX_LABEL},
{"extensionsCheckboxLabel", IDS_SETTINGS_EXTENSIONS_CHECKBOX_LABEL}, {"extensionsCheckboxLabel", IDS_SETTINGS_EXTENSIONS_CHECKBOX_LABEL},
......
...@@ -512,7 +512,7 @@ CrSettingsPeoplePageSyncControlsTest.prototype = { ...@@ -512,7 +512,7 @@ CrSettingsPeoplePageSyncControlsTest.prototype = {
'../test_browser_proxy.js', '../test_browser_proxy.js',
'sync_test_util.js', 'sync_test_util.js',
'test_sync_browser_proxy.js', 'test_sync_browser_proxy.js',
'../test_util.js', 'test_util.js',
'people_page_sync_controls_test.js', 'people_page_sync_controls_test.js',
]), ]),
}; };
......
...@@ -27,8 +27,7 @@ cr.define('settings_people_page_sync_controls', function() { ...@@ -27,8 +27,7 @@ cr.define('settings_people_page_sync_controls', function() {
}); });
test('SettingIndividualDatatypes', function() { test('SettingIndividualDatatypes', function() {
const syncAllDataTypesControl = const syncAllDataTypesControl = syncControls.$.syncAllDataTypesControl;
syncControls.$$('#syncAllDataTypesControl');
assertFalse(syncAllDataTypesControl.disabled); assertFalse(syncAllDataTypesControl.disabled);
assertTrue(syncAllDataTypesControl.checked); assertTrue(syncAllDataTypesControl.checked);
...@@ -166,82 +165,4 @@ cr.define('settings_people_page_sync_controls', function() { ...@@ -166,82 +165,4 @@ cr.define('settings_people_page_sync_controls', function() {
settings.Router.getInstance().getCurrentRoute()); settings.Router.getInstance().getCurrentRoute());
}); });
}); });
// TODO(msalama): Merge |SyncControlsFriendlySettingsOnTest| and
// |SyncControlsTest| once |syncSetupFriendlySettings| is enabled 100%.
suite('SyncControlsFriendlySettingsOnTest', function() {
let syncControls = null;
let browserProxy = null;
let syncEverything = null;
let customizeSync = null;
suiteSetup(function() {
loadTimeData.overrideValues({
syncSetupFriendlySettings: true,
});
});
setup(function() {
browserProxy = new TestSyncBrowserProxy();
settings.SyncBrowserProxyImpl.instance_ = browserProxy;
PolymerTest.clearBody();
syncControls = document.createElement('settings-sync-controls');
document.body.appendChild(syncControls);
// Start with Sync All.
cr.webUIListenerCallback(
'sync-prefs-changed', sync_test_util.getSyncAllPrefs());
Polymer.dom.flush();
return test_util.waitBeforeNextRender().then(() => {
syncEverything =
syncControls.$$('cr-radio-button[name="sync-everything"]');
customizeSync =
syncControls.$$('cr-radio-button[name="customize-sync"]');
assertTrue(!!syncEverything);
assertTrue(!!customizeSync);
});
});
teardown(function() {
syncControls.remove();
});
test('SettingIndividualDatatypes', function() {
assertTrue(syncEverything.checked);
assertFalse(customizeSync.checked);
assertEquals(syncControls.$$('#syncAllDataTypesControl'), null);
// Assert that all the individual datatype controls are disabled.
const datatypeControls = syncControls.shadowRoot.querySelectorAll(
'.list-item:not([hidden]) > cr-toggle');
for (const control of datatypeControls) {
assertTrue(control.disabled);
assertTrue(control.checked);
}
customizeSync.click();
Polymer.dom.flush();
assertFalse(syncEverything.checked);
assertTrue(customizeSync.checked);
function verifyPrefs(prefs) {
const expected = sync_test_util.getSyncAllPrefs();
expected.syncAllDataTypes = false;
assertEquals(JSON.stringify(expected), JSON.stringify(prefs));
cr.webUIListenerCallback('sync-prefs-changed', expected);
// Assert that all the individual datatype controls are enabled.
for (const control of datatypeControls) {
assertFalse(control.disabled);
assertTrue(control.checked);
}
browserProxy.resetResolver('setSyncDatatypes');
}
return browserProxy.whenCalled('setSyncDatatypes').then(verifyPrefs);
});
});
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment