Commit fecdf707 authored by James Cook's avatar James Cook Committed by Commit Bot

settings: Make Profile row link to Google Accounts on Chrome OS

By default, clicking on the People > Profile row will open the Chrome OS
account manager in a new window.

Change the row to use the "link out" arrow icon. Screenshot:
http://screen/UBHW5acnie0

If ChromeOSAccountManager is disabled, it does nothing on click, like
before.

If SplitSettings is disabled, it opens the profile picture picker, like
before.

Bug: 1004446
Test: added to browser_tests
Change-Id: Ia537bdc4fa841ec1995496b87c4650e0b1e761a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1811862Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697820}
parent 02fcc703
...@@ -181,10 +181,10 @@ ...@@ -181,10 +181,10 @@
aria-describedby="profile-name"></cr-icon-button> aria-describedby="profile-name"></cr-icon-button>
</if> </if>
<if expr="chromeos"> <if expr="chromeos">
<cr-icon-button class="subpage-arrow" <cr-icon-button class$="[[profileRowIconClass_]]"
id="profile-subpage-arrow" id="profile-subpage-arrow"
hidden="[[!isProfileActionable_]]" hidden="[[!isProfileActionable_]]"
aria-label="$i18n{changePictureTitle}" aria-label$="[[profileRowIconAriaLabel_]]"
aria-describedby="profile-name"></cr-icon-button> aria-describedby="profile-name"></cr-icon-button>
</if> </if>
</div> </div>
......
...@@ -98,8 +98,16 @@ Polymer({ ...@@ -98,8 +98,16 @@ Polymer({
isProfileActionable_: { isProfileActionable_: {
type: Boolean, type: Boolean,
value: function() { value: function() {
// On Chrome OS, only allow when SplitSettings is disabled. if (!cr.isChromeOS) {
return cr.isChromeOS ? loadTimeData.getBoolean('showOSSettings') : true; // Opens profile manager.
return true;
}
if (loadTimeData.getBoolean('showOSSettings')) {
// Pre-SplitSettings opens change picture.
return true;
}
// Post-SplitSettings links out to account manager if it is available.
return loadTimeData.getBoolean('isAccountManagerEnabled');
}, },
readOnly: true, readOnly: true,
}, },
...@@ -110,6 +118,40 @@ Polymer({ ...@@ -110,6 +118,40 @@ Polymer({
*/ */
profileName_: String, profileName_: String,
// <if expr="chromeos">
/** @private {string} */
profileRowIconClass_: {
type: String,
value: function() {
if (loadTimeData.getBoolean('showOSSettings')) {
// Pre-SplitSettings links internally to the change picture subpage.
return 'subpage-arrow';
} else {
// Post-SplitSettings links externally to account manager. If account
// manager isn't available the icon will be hidden.
return 'icon-external';
}
},
readOnly: true,
},
/** @private {string} */
profileRowIconAriaLabel_: {
type: String,
value: function() {
if (loadTimeData.getBoolean('showOSSettings')) {
// Pre-SplitSettings.
return this.i18n('changePictureTitle');
} else {
// Post-SplitSettings. If account manager isn't available the icon
// will be hidden so the label doesn't matter.
return this.i18n('accountManagerSubMenuLabel');
}
},
readOnly: true,
},
// </if>
// <if expr="not chromeos"> // <if expr="not chromeos">
/** @private {boolean} */ /** @private {boolean} */
shouldShowGoogleAccount_: { shouldShowGoogleAccount_: {
...@@ -379,10 +421,13 @@ Polymer({ ...@@ -379,10 +421,13 @@ Polymer({
/** @private */ /** @private */
onProfileTap_: function() { onProfileTap_: function() {
// <if expr="chromeos"> // <if expr="chromeos">
if (this.isProfileActionable_) { if (loadTimeData.getBoolean('showOSSettings')) {
// Testing isProfileActionable_ is simpler than conditionally removing // Pre-SplitSettings.
// on-click handlers in the HTML.
settings.navigateTo(settings.routes.CHANGE_PICTURE); settings.navigateTo(settings.routes.CHANGE_PICTURE);
} else if (loadTimeData.getBoolean('isAccountManagerEnabled')) {
// Post-SplitSettings. The browser C++ code loads OS settings in a window.
// Don't use window.open() because that creates an extra empty tab.
window.location.href = 'chrome://os-settings/accountManager';
} }
// </if> // </if>
// <if expr="not chromeos"> // <if expr="not chromeos">
......
...@@ -692,31 +692,82 @@ cr.define('settings_people_page', function() { ...@@ -692,31 +692,82 @@ cr.define('settings_people_page', function() {
peoplePage.$$('#profile-name').textContent.trim()); peoplePage.$$('#profile-name').textContent.trim());
}); });
test('clicking profile row does not open change picture page', () => { test('profile row is actionable', () => {
// Simulate a signed-in user. // Simulate a signed-in user.
sync_test_util.simulateSyncStatus({ sync_test_util.simulateSyncStatus({
signedIn: true, signedIn: true,
}); });
// Profile row items aren't actionable. // Profile row opens account manager, so the row is actionable.
const profileIcon = assert(peoplePage.$$('#profile-icon'));
assertTrue(profileIcon.hasAttribute('actionable'));
const profileRow = assert(peoplePage.$$('#profile-row'));
assertTrue(profileRow.hasAttribute('actionable'));
const subpageArrow = assert(peoplePage.$$('#profile-subpage-arrow'));
assertFalse(subpageArrow.hidden);
});
test('parental controls page is shown when enabled', () => {
// Setup button is shown and enabled.
const parentalControlsItem =
assert(peoplePage.$$('settings-parental-controls-page'));
});
});
suite('Chrome OS with account manager disabled', function() {
let peoplePage = null;
let syncBrowserProxy = null;
let profileInfoBrowserProxy = null;
suiteSetup(function() {
loadTimeData.overrideValues({
// Simulate SplitSettings (OS settings in their own surface).
showOSSettings: false,
// Disable ChromeOSAccountManager (Google Accounts support).
isAccountManagerEnabled: false,
});
});
setup(async function() {
syncBrowserProxy = new TestSyncBrowserProxy();
settings.SyncBrowserProxyImpl.instance_ = syncBrowserProxy;
profileInfoBrowserProxy = new TestProfileInfoBrowserProxy();
settings.ProfileInfoBrowserProxyImpl.instance_ =
profileInfoBrowserProxy;
PolymerTest.clearBody();
peoplePage = document.createElement('settings-people-page');
peoplePage.pageVisibility = settings.pageVisibility;
document.body.appendChild(peoplePage);
await syncBrowserProxy.whenCalled('getSyncStatus');
Polymer.dom.flush();
});
teardown(function() {
peoplePage.remove();
});
test('profile row is not actionable', () => {
// Simulate a signed-in user.
sync_test_util.simulateSyncStatus({
signedIn: true,
});
// Account manager isn't available, so the row isn't actionable.
const profileIcon = assert(peoplePage.$$('#profile-icon')); const profileIcon = assert(peoplePage.$$('#profile-icon'));
assertFalse(profileIcon.hasAttribute('actionable')); assertFalse(profileIcon.hasAttribute('actionable'));
const profileRow = assert(peoplePage.$$('#profile-row')); const profileRow = assert(peoplePage.$$('#profile-row'));
assertFalse(profileRow.hasAttribute('actionable')); assertFalse(profileRow.hasAttribute('actionable'));
const subpageArrow = assert(peoplePage.$$('#profile-subpage-arrow')); const subpageArrow = assert(peoplePage.$$('#profile-subpage-arrow'));
assertFalse(subpageArrow.hasAttribute('actionable')); assertTrue(subpageArrow.hidden);
// Clicking on profile icon doesn't navigate to a new route. // Clicking on profile icon doesn't navigate to a new route.
const oldRoute = settings.getCurrentRoute(); const oldRoute = settings.getCurrentRoute();
profileIcon.click(); profileIcon.click();
assertEquals(oldRoute, settings.getCurrentRoute()); assertEquals(oldRoute, settings.getCurrentRoute());
}); });
test('parental controls page is shown when enabled', () => {
// Setup button is shown and enabled.
const parentalControlsItem =
assert(peoplePage.$$('settings-parental-controls-page'));
});
}); });
/** @implements {parental_controls.ParentalControlsBrowserProxy} */ /** @implements {parental_controls.ParentalControlsBrowserProxy} */
......
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