Commit 874dd354 authored by Daniel Classon's avatar Daniel Classon Committed by Commit Bot

[OsSettingsPeoplePage] Combine sync enabled and disabled buttons

Combine the OS Sync Controls "turn on sync" and "turn off sync" buttons.
This is better for a11y as the button doesn't lose focus when clicked.
This is related to settings deep linking as its easier to deep link to
a single element rather than two elements sharing one setting.

Bug: 1084154
Change-Id: I545498954934368f47c8f6af168830014e41947d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359164
Commit-Queue: Daniel Classon <dclasson@google.com>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798964}
parent 597dd6df
......@@ -120,20 +120,12 @@
</div>
</div>
<cr-button id="turnOnSyncButton"
<cr-button id="syncOnOffButton"
class="action-button"
on-click="onTurnOnSyncButtonClick_"
aria-labelledby="turnOnSyncButton accountTitle accountSubtitle"
aria-describedby="featureLabel"
hidden="[[osSyncFeatureEnabled]]">
$i18n{osSyncTurnOn}
</cr-button>
<cr-button id="turnOffSyncButton"
on-click="onTurnOffSyncButtonClick_"
aria-labelledby="turnOffSyncButton accountTitle accountSubtitle"
aria-describedby="featureLabel"
hidden="[[!osSyncFeatureEnabled]]">
$i18n{osSyncTurnOff}
on-click="onSyncOnOffButtonClick_"
aria-labelledby="syncOnOffButton accountTitle accountSubtitle"
aria-describedby="featureLabel">
[[getSyncOnOffButtonLabel_(osSyncFeatureEnabled)]]
</cr-button>
</div>
......
......@@ -150,6 +150,17 @@ Polymer({
this.profileEmail;
},
/**
* @return {string}
* @private
*/
getSyncOnOffButtonLabel_() {
if (!this.osSyncFeatureEnabled) {
return this.i18n('osSyncTurnOn');
}
return this.i18n('osSyncTurnOff');
},
/**
* Returns the CSS class for the sync status icon.
* @return {string}
......@@ -216,14 +227,8 @@ Polymer({
},
/** @private */
onTurnOnSyncButtonClick_() {
this.browserProxy_.setOsSyncFeatureEnabled(true);
settings.recordSettingChange();
},
/** @private */
onTurnOffSyncButtonClick_() {
this.browserProxy_.setOsSyncFeatureEnabled(false);
onSyncOnOffButtonClick_() {
this.browserProxy_.setOsSyncFeatureEnabled(!this.osSyncFeatureEnabled);
settings.recordSettingChange();
},
......
......@@ -198,8 +198,7 @@ suite('OsSyncControlsTest', function() {
test('FeatureDisabled', function() {
setupWithFeatureDisabled();
assertFalse(syncControls.$.turnOnSyncButton.hidden);
assertTrue(syncControls.$.turnOffSyncButton.hidden);
assertTrue(!!syncControls.$.syncOnOffButton);
assertTrue(syncControls.$.syncEverythingCheckboxLabel.hasAttribute(
'label-disabled'));
......@@ -225,8 +224,7 @@ suite('OsSyncControlsTest', function() {
test('FeatureEnabled', function() {
setupWithFeatureEnabled();
assertTrue(syncControls.$.turnOnSyncButton.hidden);
assertFalse(syncControls.$.turnOffSyncButton.hidden);
assertTrue(!!syncControls.$.syncOnOffButton);
assertFalse(syncControls.$.syncEverythingCheckboxLabel.hasAttribute(
'label-disabled'));
......@@ -251,14 +249,14 @@ suite('OsSyncControlsTest', function() {
test('ClickingTurnOffDisablesFeature', async function() {
setupWithFeatureEnabled();
syncControls.$.turnOffSyncButton.click();
syncControls.$.syncOnOffButton.click();
const enabled = await browserProxy.whenCalled('setOsSyncFeatureEnabled');
assertFalse(enabled);
});
test('ClickingTurnOnEnablesFeature', async function() {
setupWithFeatureDisabled();
syncControls.$.turnOnSyncButton.click();
syncControls.$.syncOnOffButton.click();
enabled = await browserProxy.whenCalled('setOsSyncFeatureEnabled');
assertTrue(enabled);
});
......
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