[UNO] Update sync controls disabled/managed state [chromium/src : main]
0 views
Skip to first unread message
gwsq (Gerrit)
Sep 23, 2025, 9:29:02 AM (3 days ago) Sep 23
to Amelie Schneider, Chrome Signin Team, Ryan Sultanem, Arthur Milchior, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Arthur Milchior and Ryan Sultanem
Message from gwsq![]( "Open in Gerrit")
Shadowed: arthurm...@google.com
Reviewer source(s):
arthurm...@google.com, rs...@google.com is from context(googleclient/chrome/chromium_gwsq/components/signin/config.gwsq)
Related details
Attention is currently required from:
- Arthur Milchior
- Ryan Sultanem
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arthur Milchior (Gerrit)
Sep 23, 2025, 11:35:16 AM (2 days ago) Sep 23
to Amelie Schneider, Chrome Signin Team, Ryan Sultanem, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Amelie Schneider and Ryan Sultanem
Arthur Milchior added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 4 (Latest):
Arthur Milchior . unresolved
Hi Amelie,
Nice to meet you
How did you end up selecting me. I work exclusively on iOS, I’ve no context about what those files are. Please find someone relevant on this domain.
Related details
Attention is currently required from:
- Amelie Schneider
- Ryan Sultanem
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Amelie Schneider (Gerrit)
Sep 23, 2025, 11:46:27 AM (2 days ago) Sep 23
to Chrome Signin Team, Ryan Sultanem, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Ryan Sultanem
Amelie Schneider added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Arthur Milchior . resolved
Hi Amelie,
Nice to meet you
How did you end up selecting me. I work exclusively on iOS, I’ve no context about what those files are. Please find someone relevant on this domain.
Amelie Schneider
Discussed offline, added through chrome-signin-reviews.
Related details
Attention is currently required from:
- Ryan Sultanem
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ryan Sultanem (Gerrit)
Sep 24, 2025, 2:40:25 AM (yesterday) Sep 24
to Amelie Schneider, Chrome Signin Team, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Amelie Schneider
Ryan Sultanem voted and added 3 comments![]( "Open in Gerrit")
Votes added by Ryan Sultanem
| Code-Review | +1 |
3 comments
Patchset-level comments
Ryan Sultanem . resolved
LGTM thanks!
File chrome/browser/resources/settings/people_page/sync_controls.ts
Line 277, Patchset 4 (Latest):
return syncAllDataTypes || dataTypeManaged;Ryan Sultanem . unresolved
optional:
Could we move this part of the check (along with the one below) in the above condition? Or in a separate one.
The function would like this:
- If no `syncStatus`: return true because we cannot deduce the value
- If `dataTypeManaged`: return true because the user should not have access to type state at all.
- If `SignedInState.SYNCING`: return if the user is syncing all types. (I assume this is equivalent to the previous `if (!this.isAccountSettingsPage_)`; if so this is a good change for later cleanup, nice!.
- The rest with `sync.disabled`.
IMO it reads a bit better, each condition is properly separated with it's return value logic.
File chrome/test/data/webui/settings/people_page_sync_controls_test.ts
Line 26, Patchset 4 (Latest):
Ryan Sultanem . unresolved
Not sure if it already exists; is it possible to add a test that checks your change?
E.g. loading the account setting page for signed in accounts, then triggering a state change with `sync.disabled` value change, and ensuring that the toggles react as expected?
Related details
Attention is currently required from:
- Amelie Schneider
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ryan Sultanem (Gerrit)
Sep 24, 2025, 2:44:54 AM (yesterday) Sep 24
to Amelie Schneider, Chrome Signin Team, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Amelie Schneider
Ryan Sultanem added 1 comment![]( "Open in Gerrit")
File chrome/browser/resources/settings/people_page/sync_controls.ts
Line 269, Patchset 4 (Latest):
private disableTypeCheckBox_(Ryan Sultanem . unresolved
Should this function also react to `syncStatus.disabled` changes? In the HTML file, otherwise it is actually not very clear to me how your change is allowing for auto update/reaction of the toggle states.
Could you please help me understand this part? THanks
Amelie Schneider (Gerrit)
Sep 24, 2025, 8:39:03 AM (yesterday) Sep 24
to Ryan Sultanem, Chrome Signin Team, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Ryan Sultanem
Amelie Schneider added 4 comments![]( "Open in Gerrit")
Patchset-level comments
File chrome/browser/resources/settings/people_page/sync_controls.ts
Line 269, Patchset 4:
private disableTypeCheckBox_(Ryan Sultanem . unresolved
Should this function also react to `syncStatus.disabled` changes? In the HTML file, otherwise it is actually not very clear to me how your change is allowing for auto update/reaction of the toggle states.
Could you please help me understand this part? THanks
Amelie Schneider
It reacts to the `syncStatus` in the .html file, does it need to be `syncStatus.disabled` specifically?
optional:
Could we move this part of the check (along with the one below) in the above condition? Or in a separate one.
The function would like this:
- If no `syncStatus`: return true because we cannot deduce the value
- If `dataTypeManaged`: return true because the user should not have access to type state at all.
- If `SignedInState.SYNCING`: return if the user is syncing all types. (I assume this is equivalent to the previous `if (!this.isAccountSettingsPage_)`; if so this is a good change for later cleanup, nice!.
- The rest with `sync.disabled`.
IMO it reads a bit better, each condition is properly separated with it's return value logic.
Amelie Schneider
Done
File chrome/test/data/webui/settings/people_page_sync_controls_test.ts
Ryan Sultanem . unresolved
Not sure if it already exists; is it possible to add a test that checks your change?
E.g. loading the account setting page for signed in accounts, then triggering a state change with `sync.disabled` value change, and ensuring that the toggles react as expected?
Amelie Schneider
This is what happens in `DisableToggleAndHidePolicyIndicatorWhenSyncIsDisabled`: https://source.chromium.org/chromium/chromium/src/+/main:chrome/test/data/webui/settings/people_page_sync_controls_test.ts;drc=2cd3dd8e08129240085ccaa4a5123fd44374e693;l=654
Related details
Attention is currently required from:
- Ryan Sultanem
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ryan Sultanem (Gerrit)
Sep 24, 2025, 9:53:35 AM (yesterday) Sep 24
to Amelie Schneider, Chrome Signin Team, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Amelie Schneider
Ryan Sultanem added 1 comment![]( "Open in Gerrit")
Attention is currently required from:
- Amelie Schneider
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Amelie Schneider (Gerrit)
Sep 24, 2025, 11:25:10 AM (yesterday) Sep 24
to Ryan Sultanem, Chrome Signin Team, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Ryan Sultanem
Amelie Schneider added 1 comment![]( "Open in Gerrit")
File chrome/browser/resources/settings/people_page/sync_controls.ts
Line 269, Patchset 4:
private disableTypeCheckBox_(Ryan Sultanem . resolved
Amelie SchneiderShould this function also react to `syncStatus.disabled` changes? In the HTML file, otherwise it is actually not very clear to me how your change is allowing for auto update/reaction of the toggle states.
Could you please help me understand this part? THanks
It reacts to the `syncStatus` in the .html file, does it need to be `syncStatus.disabled` specifically?
Ryan Sultanem
Amelie SchneiderUsually, I believe it is preferred to react directly to what it depends on, in this case both `syncStatus` and `syncStatus.disabled`.
I am not sure if this is the case here or not, but if `syncStatus.disabled` was set without resetting the `syncStatus` itself, I believe your function would not react accordingly.
Okay, added `syncStatus.disabled`.
Related details
Attention is currently required from:
- Ryan Sultanem
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ryan Sultanem (Gerrit)
Sep 24, 2025, 11:27:24 AM (yesterday) Sep 24
to Amelie Schneider, Chrome Signin Team, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Amelie Schneider
Ryan Sultanem voted and added 1 comment![]( "Open in Gerrit")
Votes added by Ryan Sultanem
| Code-Review | +1 |
Attention is currently required from:
- Amelie Schneider
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ryan Sultanem (Gerrit)
Sep 24, 2025, 11:29:09 AM (yesterday) Sep 24
to Amelie Schneider, Chrome Signin Team, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Amelie Schneider
Ryan Sultanem added 1 comment![]( "Open in Gerrit")
File chrome/browser/resources/settings/people_page/sync_controls.ts
Line 270, Patchset 6 (Latest):
syncStatus: SyncStatus, syncDisabled: boolean, syncAllDataTypes: boolean,Ryan Sultanem . resolved
FYI: you could add `syncStatus.disabled` at the end of the list of the callers for it to be a reactive property, without it necessarily being an input parameter as well.
Related details
Attention is currently required from:
- Amelie Schneider
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Amelie Schneider (Gerrit)
Sep 24, 2025, 11:31:09 AM (yesterday) Sep 24
to Ryan Sultanem, Chrome Signin Team, Chromium LUCI CQ, chromium...@chromium.org
Amelie Schneider voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Amelie Schneider (Gerrit)
6:43 AM (17 hours ago) 6:43 AM
to Ryan Sultanem, Chrome Signin Team, Chromium LUCI CQ, chromium...@chromium.org
Amelie Schneider voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Amelie Schneider (Gerrit)
12:03 PM (11 hours ago) 12:03 PM
to Ryan Sultanem, Chrome Signin Team, Chromium LUCI CQ, chromium...@chromium.org
Amelie Schneider voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
12:06 PM (11 hours ago) 12:06 PM
to Amelie Schneider, Ryan Sultanem, Chrome Signin Team, chromium...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
6 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: chrome/browser/resources/settings/people_page/sync_controls.html
Insertions: 49, Deletions: 34.
@@ -88,8 +88,8 @@
checked="[[mergedHistoryTabsToggleChecked_(syncPrefs)]]"
on-change="onMergedHistoryTabsToggleChanged_"
disabled="[[mergedHistoryTabsToggleDisabled_(syncStatus,
- syncStatus.disabled, syncPrefs.tabsManaged,
- syncPrefs.typedUrlsManaged)]]"
+ syncPrefs.tabsManaged, syncPrefs.typedUrlsManaged,
+ syncStatus.disabled)]]"
aria-labelledby="historyTabsCheckboxLabel">
</cr-toggle>
</div>
@@ -108,8 +108,9 @@
<cr-toggle id="historyToggle"
checked="{{syncPrefs.typedUrlsSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[mergedHistoryTabsToggleDisabled_(syncStatus,
- syncStatus.disabled, syncPrefs.typedUrlsManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes, syncPrefs.typedUrlsManaged,
+ syncStatus.disabled)]]"
aria-labelledby="historyCheckboxLabel"
data-type$="[[UserSelectableTypeEnum_.HISTORY]]">
</cr-toggle>
@@ -126,8 +127,9 @@
</cr-policy-indicator>
<cr-toggle checked="{{syncPrefs.bookmarksSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[disableTypeCheckBox_(syncStatus, syncStatus.disabled,
- syncPrefs.syncAllDataTypes, syncPrefs.bookmarksManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes, syncPrefs.bookmarksManaged,
+ syncStatus.disabled)]]"
aria-labelledby="bookmarksCheckboxLabel"
data-type$="[[UserSelectableTypeEnum_.BOOKMARKS]]">
</cr-toggle>
@@ -143,8 +145,9 @@
</cr-policy-indicator>
<cr-toggle checked="{{syncPrefs.readingListSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[disableTypeCheckBox_(syncStatus, syncStatus.disabled,
- syncPrefs.syncAllDataTypes ,syncPrefs.readingListManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes ,syncPrefs.readingListManaged,
+ syncStatus.disabled)]]"
aria-labelledby="readingListCheckboxLabel"
data-type$="[[UserSelectableTypeEnum_.READING_LIST]]">
</cr-toggle>
@@ -161,8 +164,9 @@
</cr-policy-indicator>
<cr-toggle checked="{{syncPrefs.tabsSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[disableTypeCheckBox_(syncStatus, syncStatus.disabled,
- syncPrefs.syncAllDataTypes,syncPrefs.tabsManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes,syncPrefs.tabsManaged,
+ syncStatus.disabled)]]"
aria-labelledby="openTabsCheckboxLabel"
data-type$="[[UserSelectableTypeEnum_.TABS]]">
</cr-toggle>
@@ -173,13 +177,14 @@
$i18n{savedTabGroupsCheckboxLabel}
</div>
<cr-policy-indicator indicator-type="userPolicy"
- hidden="[[!showPolicyIndicator_(syncStatus, syncStatus.disabled,
- syncPrefs.savedTabGroupsManaged)]]">
+ hidden="[[!showPolicyIndicator_(syncStatus,
+ syncPrefs.savedTabGroupsManaged, syncStatus.disabled)]]">
</cr-policy-indicator>
<cr-toggle checked="{{syncPrefs.savedTabGroupsSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[disableTypeCheckBox_(syncStatus, syncStatus.disabled,
- syncPrefs.syncAllDataTypes, syncPrefs.savedTabGroupsManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes, syncPrefs.savedTabGroupsManaged,
+ syncStatus.disabled)]]"
aria-labelledby="savedTabGroupsCheckboxLabel"
data-type$="[[UserSelectableTypeEnum_.SAVED_TAB_GROUPS]]">
</cr-toggle>
@@ -197,8 +202,9 @@
<cr-toggle id="autofillCheckbox"
checked="{{syncPrefs.autofillSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[disableTypeCheckBox_(syncStatus, syncStatus.disabled,
- syncPrefs.syncAllDataTypes,syncPrefs.autofillManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes,syncPrefs.autofillManaged,
+ syncStatus.disabled)]]"
aria-labelledby="autofillCheckboxLabel"
data-type$="[[UserSelectableTypeEnum_.AUTOFILL]]">
</cr-toggle>
@@ -216,8 +222,9 @@
<cr-toggle id="paymentsCheckbox"
checked="{{syncPrefs.paymentsSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[disableTypeCheckBox_(syncStatus, syncStatus.disabled,
- syncPrefs.syncAllDataTypes, syncPrefs.paymentsManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes, syncPrefs.paymentsManaged,
+ syncStatus.disabled)]]"
aria-label="$i18n{paymentsCheckboxLabel}"
data-type$="[[UserSelectableTypeEnum_.PAYMENTS]]">
</cr-toggle>
@@ -233,8 +240,9 @@
</cr-policy-indicator>
<cr-toggle checked="{{syncPrefs.passwordsSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[disableTypeCheckBox_(syncStatus, syncStatus.disabled,
- syncPrefs.syncAllDataTypes, syncPrefs.passwordsManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes, syncPrefs.passwordsManaged,
+ syncStatus.disabled)]]"
aria-labelledby="passwordsCheckboxLabel"
data-type$="[[UserSelectableTypeEnum_.PASSWORDS]]">
</cr-toggle>
@@ -250,8 +258,9 @@
</cr-policy-indicator>
<cr-toggle checked="{{syncPrefs.preferencesSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[disableTypeCheckBox_(syncStatus, syncStatus.disabled,
- syncPrefs.syncAllDataTypes, syncPrefs.preferencesManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes, syncPrefs.preferencesManaged,
+ syncStatus.disabled)]]"
aria-labelledby="settingsCheckboxLabel"
data-type$="[[UserSelectableTypeEnum_.PREFERENCES]]">
</cr-toggle>
@@ -265,8 +274,9 @@
</cr-policy-indicator>
<cr-toggle checked="{{syncPrefs.appsSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[disableTypeCheckBox_(syncStatus, syncStatus.disabled,
- syncPrefs.syncAllDataTypes, syncPrefs.appsManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes, syncPrefs.appsManaged,
+ syncStatus.disabled)]]"
aria-labelledby="appCheckboxLabel"
data-type$="[[UserSelectableTypeEnum_.APPS]]">
</cr-toggle>
@@ -282,8 +292,9 @@
</cr-policy-indicator>
<cr-toggle checked="{{syncPrefs.extensionsSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[disableTypeCheckBox_(syncStatus, syncStatus.disabled,
- syncPrefs.syncAllDataTypes , syncPrefs.extensionsManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes , syncPrefs.extensionsManaged,
+ syncStatus.disabled)]]"
aria-labelledby="extensionsCheckboxLabel"
data-type$="[[UserSelectableTypeEnum_.EXTENSIONS]]">
</cr-toggle>
@@ -299,8 +310,9 @@
</cr-policy-indicator>
<cr-toggle checked="{{syncPrefs.themesSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[disableTypeCheckBox_(syncStatus, syncStatus.disabled,
- syncPrefs.syncAllDataTypes ,syncPrefs.themesManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes ,syncPrefs.themesManaged,
+ syncStatus.disabled)]]"
aria-labelledby="themeCheckboxLabel"
data-type$="[[UserSelectableTypeEnum_.THEMES]]">
</cr-toggle>
@@ -317,8 +329,9 @@
</cr-policy-indicator>
<cr-toggle checked="{{syncPrefs.productComparisonSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[disableTypeCheckBox_(syncStatus, syncStatus.disabled,
- syncPrefs.syncAllDataTypes, syncPrefs.productComparisonManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes, syncPrefs.productComparisonManaged,
+ syncStatus.disabled)]]"
aria-labelledby="productComparisonsCheckboxLabel"
data-type$="[[UserSelectableTypeEnum_.PRODUCT_COMPARISON]]">
</cr-toggle>
@@ -337,8 +350,9 @@
<cr-toggle id="cookiesCheckbox"
checked="{{syncPrefs.cookiesSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[disableTypeCheckBox_(syncStatus, syncStatus.disabled,
- syncPrefs.syncAllDataTypes, syncPrefs.cookiesManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes, syncPrefs.cookiesManaged,
+ syncStatus.disabled)]]"
aria-label="$i18n{cookiesCheckboxLabel}">
</cr-toggle>
</div>
@@ -354,8 +368,9 @@
</cr-policy-indicator>
<cr-toggle checked="{{syncPrefs.wifiConfigurationsSynced}}"
on-change="onSingleSyncDataTypeChanged_"
- disabled="[[disableTypeCheckBox_(syncStatus, syncStatus.disabled,
- syncPrefs.syncAllDataTypes,syncPrefs.wifiConfigurationsManaged)]]"
+ disabled="[[disableTypeCheckBox_(syncStatus,
+ syncPrefs.syncAllDataTypes,syncPrefs.wifiConfigurationsManaged,
+ syncStatus.disabled)]]"
aria-labelledby="wifiConfigurationsCheckboxLabel">
</cr-toggle>
</div>
```
```
The name of the file: chrome/test/data/webui/settings/people_page_sync_controls_test.ts
Insertions: 0, Deletions: 56.
@@ -462,62 +462,6 @@
});
test(
- 'DisableToggleAndHidePolicyIndicatorWhenSyncPrefsNotLoaded', async () => {
- webUIListenerCallback('sync-prefs-changed', undefined);
- await flushTasks();
- await waitAfterNextRender(syncControls);
-
- // Controls are still available when prefs are not loaded.
- assertFalse(syncControls.hidden);
-
- // However, they are disabled.
- assertControlsEnabled(false);
-
- // Assert that all policy indicators are hidden.
- assertSyncDisabledPolicyIndicatorShown(false);
- assertIndividualItemPolicyIndicatorsShown(false);
- });
-
- test('DisableToggleAndHidePolicyIndicatorWhenSyncIsDisabled', async () => {
- setupPrefs();
-
- syncControls.syncStatus = {
- disabled: true,
- hasError: false,
- signedInState: SignedInState.SIGNED_IN,
- statusAction: StatusAction.NO_ACTION,
- };
- await waitAfterNextRender(syncControls);
-
- // Controls are still available when sync is disabled.
- assertFalse(syncControls.hidden);
-
- // However, they are disabled.
- assertControlsEnabled(false);
-
- // Assert that only the sync disabled policy indicator is shown.
- assertSyncDisabledPolicyIndicatorShown(true);
- assertIndividualItemPolicyIndicatorsShown(false);
- });
-
- test('DisableToggleAndShowPolicyIndicatorWhenDataTypeIsManaged', async () => {
- // Set all prefs to managed.
- webUIListenerCallback('sync-prefs-changed', getSyncAllPrefsManaged());
- await flushTasks();
- await waitAfterNextRender(syncControls);
-
- // Controls are still available when data types are managed.
- assertFalse(syncControls.hidden);
-
- // However, they are disabled.
- assertControlsEnabled(false);
-
- // Assert that only individual items' policy indicators are shown.
- assertSyncDisabledPolicyIndicatorShown(false);
- assertIndividualItemPolicyIndicatorsShown(true);
- });
-
- test(
'DisableMergedToggleAndShowPolicyIndicatorWhenHistoryAndTabsManaged',
async () => {
// Set history and tabs to managed.
```
```
The name of the file: chrome/browser/resources/settings/people_page/sync_controls.ts
Insertions: 4, Deletions: 4.
@@ -190,9 +190,9 @@
}
private mergedHistoryTabsToggleDisabled_(
- syncStatus: SyncStatus, syncDisabled: boolean, tabsManaged: boolean,
+ syncStatus: SyncStatus, tabsManaged: boolean,
historyManaged: boolean): boolean {
- return !syncStatus || syncDisabled || !this.syncPrefs ||
+ return !syncStatus || syncStatus.disabled || !this.syncPrefs ||
(tabsManaged && historyManaged);
}
@@ -267,7 +267,7 @@
}
private disableTypeCheckBox_(
- syncStatus: SyncStatus, syncDisabled: boolean, syncAllDataTypes: boolean,
+ syncStatus: SyncStatus, syncAllDataTypes: boolean,
dataTypeManaged: boolean): boolean {
if (!syncStatus) {
return true;
@@ -284,7 +284,7 @@
// Toggles should be disabled on the account settings page if sync is
// disabled, or if the sync prefs are undefined, which is the case e.g.
// right after startup.
- return syncDisabled || !this.syncPrefs;
+ return syncStatus.disabled || !this.syncPrefs;
}
private showPolicyIndicator_(
```
Change information
Commit message:
[UNO] Update sync controls disabled/managed state
Until now, the toggles on the new account settings page were not
properly updated with sync disabled, as they were loaded before the
account settings page was shown and did not react to sync disabled. The
user had to refresh the pages in order for them to be properly disabled.
With this change, the toggles react to the signed in state rather than
the current route, which means that they are disabled with sync disabled
whenever the user is not syncing.
Additionally, the sync disabled information reacts to the current route.
This only affects changes behind the feature flag
`ReplaceSyncPromosWithSignInPromos`.
Bug: 446117071
Change-Id: I096ad0a69aee4054e34d3301881a2ce50c464be6
Reviewed-by: Ryan Sultanem <rs...@google.com>
Commit-Queue: Amelie Schneider <ame...@google.com>
Cr-Commit-Position: refs/heads/main@{#1520654}
Files:
- M chrome/browser/resources/settings/people_page/sync_controls.html
- M chrome/browser/resources/settings/people_page/sync_controls.ts
- M chrome/test/data/webui/settings/people_page_sync_controls_test.ts
Change size: M
Delta: 3 files changed, 56 insertions(+), 80 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Ryan Sultanem
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages