Commit c0453c6a authored by Xinghui Lu's avatar Xinghui Lu Committed by Commit Bot

Add secondary CTA in SafetyCheck rows.

This change affects the password check row, the Safe Browsing row, and
the extension row. When the state is SAFE, make the entire row clickable
and display a clickable icon in the row. When the row is clicked, direct
the user to its corresponding settings page.

before: http://screen/ArPirizGoZhfiYz
after: http://screen/76CN4UQ4a4rbk4p
screen record: http://dr/file/d/1E6tBzL62Yfci7M4QqcWQm25cRX8O_7xl/view

Bug: 1103015
Change-Id: I545207019b52962e0b53dd1455b2e1108c4e2b20
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2401812Reviewed-by: default avatarRainhard Findling <rainhard@chromium.org>
Reviewed-by: default avatarVarun Khaneja <vakh@chromium.org>
Reviewed-by: default avatarTheodore Olsauskas-Warren <sauski@google.com>
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809000}
parent a9a01c37
...@@ -31,6 +31,7 @@ js_library("safety_check_browser_proxy") { ...@@ -31,6 +31,7 @@ js_library("safety_check_browser_proxy") {
js_library("safety_check_child") { js_library("safety_check_child") {
deps = [ deps = [
"//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled", "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
"//ui/webui/resources/cr_elements/cr_icon_button:cr_icon_button.m",
"//ui/webui/resources/js:assert.m", "//ui/webui/resources/js:assert.m",
"//ui/webui/resources/js:i18n_behavior.m", "//ui/webui/resources/js:i18n_behavior.m",
] ]
......
<style include="cr-shared-style settings-shared iron-flex"> <style include="cr-shared-style settings-shared iron-flex">
:host([row-clickable]) .cr-row {
cursor: pointer;
}
iron-icon { iron-icon {
display: flex; display: flex;
flex-shrink: 0; flex-shrink: 0;
...@@ -46,6 +50,13 @@ ...@@ -46,6 +50,13 @@
[[buttonLabel]] [[buttonLabel]]
</cr-button> </cr-button>
</template> </template>
<template is="dom-if" if="[[rowClickable]]">
<cr-icon-button id="rowClickableIndicator"
iron-icon="[[getRowClickableIcon_(external)]]"
aria-describedby="subLabel"
aria-labelledby="label">
</cr-icon-button>
</template>
<template is="dom-if" if="[[showManagedIcon_(managedIcon)]]" restamp> <template is="dom-if" if="[[showManagedIcon_(managedIcon)]]" restamp>
<iron-icon id="managedIcon" icon="[[managedIcon]]" aria-hidden="true"> <iron-icon id="managedIcon" icon="[[managedIcon]]" aria-hidden="true">
</iron-icon> </iron-icon>
......
...@@ -62,6 +62,19 @@ Polymer({ ...@@ -62,6 +62,19 @@ Polymer({
// Classes of the right hand button. // Classes of the right hand button.
buttonClass: String, buttonClass: String,
// Should the entire row be clickable.
rowClickable: {
type: Boolean,
value: false,
reflectToAttribute: true,
},
// Is the row directed to external link.
external: {
type: Boolean,
value: false,
},
// Right hand managed icon. |null| removes it from the DOM. // Right hand managed icon. |null| removes it from the DOM.
managedIcon: String, managedIcon: String,
}, },
...@@ -157,4 +170,13 @@ Polymer({ ...@@ -157,4 +170,13 @@ Polymer({
showManagedIcon_: function() { showManagedIcon_: function() {
return !!this.managedIcon; return !!this.managedIcon;
}, },
/**
* Return the icon to show when the row is clickable.
* @return {string}
* @private
*/
getRowClickableIcon_() {
return this.external ? 'cr:open-in-new' : 'cr:arrow-right';
},
}); });
...@@ -7,5 +7,8 @@ ...@@ -7,5 +7,8 @@
button-aria-label="$i18n{safetyCheckExtensionsButtonAriaLabel}" button-aria-label="$i18n{safetyCheckExtensionsButtonAriaLabel}"
button-class="[[getButtonClass_(status_)]]" button-class="[[getButtonClass_(status_)]]"
on-button-click="onButtonClick_" on-button-click="onButtonClick_"
on-click="onRowClick_"
row-clickable="[[isRowClickable_(status_)]]"
external
managed-icon="[[getManagedIcon_(status_)]]"> managed-icon="[[getManagedIcon_(status_)]]">
</settings-safety-check-child> </settings-safety-check-child>
...@@ -51,6 +51,20 @@ Polymer({ ...@@ -51,6 +51,20 @@ Polymer({
* @private * @private
*/ */
displayString_: String, displayString_: String,
/**
* A set of statuses that the entire row is clickable.
* @type {!Set<!SafetyCheckExtensionsStatus>}
* @private
*/
rowClickableStatuses: {
readOnly: true,
type: Object,
value: () => new Set([
SafetyCheckExtensionsStatus.NO_BLOCKLISTED_EXTENSIONS,
SafetyCheckExtensionsStatus.ERROR,
]),
},
}, },
/** @private {?MetricsBrowserProxy} */ /** @private {?MetricsBrowserProxy} */
...@@ -133,8 +147,7 @@ Polymer({ ...@@ -133,8 +147,7 @@ Polymer({
SafetyCheckInteractions.SAFETY_CHECK_EXTENSIONS_REVIEW); SafetyCheckInteractions.SAFETY_CHECK_EXTENSIONS_REVIEW);
this.metricsBrowserProxy_.recordAction( this.metricsBrowserProxy_.recordAction(
'Settings.SafetyCheck.ReviewExtensions'); 'Settings.SafetyCheck.ReviewExtensions');
this.openExtensionsPage_();
OpenWindowProxyImpl.getInstance().openURL('chrome://extensions');
}, },
/** /**
...@@ -149,4 +162,27 @@ Polymer({ ...@@ -149,4 +162,27 @@ Polymer({
return null; return null;
} }
}, },
/**
* @private
* @return {?boolean}
*/
isRowClickable_: function() {
return this.rowClickableStatuses.has(this.status_);
},
/** @private */
onRowClick_: function() {
if (this.isRowClickable_()) {
// TODO(crbug.com/1103015): Log action and histogram:
// SafetyCheckInteractions.SAFETY_CHECK_EXTENSIONS_NAVIGATE
// Settings.SafetyCheck.NavigateToExtensions
this.openExtensionsPage_();
}
},
/** @private */
openExtensionsPage_: function() {
OpenWindowProxyImpl.getInstance().openURL('chrome://extensions');
},
}); });
...@@ -6,5 +6,7 @@ ...@@ -6,5 +6,7 @@
button-label="[[getButtonLabel_(status_)]]" button-label="[[getButtonLabel_(status_)]]"
button-aria-label="$i18n{safetyCheckPasswordsButtonAriaLabel}" button-aria-label="$i18n{safetyCheckPasswordsButtonAriaLabel}"
button-class="action-button" button-class="action-button"
on-button-click="onButtonClick_"> on-button-click="onButtonClick_"
on-click="onRowClick_"
row-clickable="[[isRowClickable_(status_)]]">
</settings-safety-check-child> </settings-safety-check-child>
...@@ -53,6 +53,21 @@ Polymer({ ...@@ -53,6 +53,21 @@ Polymer({
* @private * @private
*/ */
displayString_: String, displayString_: String,
/**
* A set of statuses that the entire row is clickable.
* @type {!Set<!SafetyCheckPasswordsStatus>}
* @private
*/
rowClickableStatuses: {
readOnly: true,
type: Object,
value: () => new Set([
SafetyCheckPasswordsStatus.SAFE,
SafetyCheckPasswordsStatus.QUOTA_LIMIT,
SafetyCheckPasswordsStatus.ERROR,
]),
},
}, },
/** @private {?MetricsBrowserProxy} */ /** @private {?MetricsBrowserProxy} */
...@@ -121,7 +136,29 @@ Polymer({ ...@@ -121,7 +136,29 @@ Polymer({
SafetyCheckInteractions.SAFETY_CHECK_PASSWORDS_MANAGE); SafetyCheckInteractions.SAFETY_CHECK_PASSWORDS_MANAGE);
this.metricsBrowserProxy_.recordAction( this.metricsBrowserProxy_.recordAction(
'Settings.SafetyCheck.ManagePasswords'); 'Settings.SafetyCheck.ManagePasswords');
this.openPasswordCheckPage_();
},
/**
* @private
* @return {?boolean}
*/
isRowClickable_: function() {
return this.rowClickableStatuses.has(this.status_);
},
/** @private */
onRowClick_: function() {
if (this.isRowClickable_()) {
// TODO(crbug.com/1103015): Log action and histogram:
// SafetyCheckInteractions.SAFETY_CHECK_PASSWORDS_NAVIGATE
// Settings.SafetyCheck.NavigateToPasswords
this.openPasswordCheckPage_();
}
},
/** @private */
openPasswordCheckPage_: function() {
Router.getInstance().navigateTo( Router.getInstance().navigateTo(
routes.CHECK_PASSWORDS, routes.CHECK_PASSWORDS,
/* dynamicParams= */ null, /* removeSearch= */ true); /* dynamicParams= */ null, /* removeSearch= */ true);
......
...@@ -7,5 +7,7 @@ ...@@ -7,5 +7,7 @@
button-aria-label="$i18n{safetyCheckSafeBrowsingButtonAriaLabel}" button-aria-label="$i18n{safetyCheckSafeBrowsingButtonAriaLabel}"
button-class="action-button" button-class="action-button"
on-button-click="onButtonClick_" on-button-click="onButtonClick_"
managed-icon="[[getManagedIcon_(status_)]]"> on-click="onRowClick_"
row-clickable="[[isRowClickable_(status_)]]"
managed-icon="[[getManagedIcon_(status_)]]"">
</settings-safety-check-child> </settings-safety-check-child>
...@@ -52,6 +52,21 @@ Polymer({ ...@@ -52,6 +52,21 @@ Polymer({
* @private * @private
*/ */
displayString_: String, displayString_: String,
/**
* A set of statuses that the entire row is clickable.
* @type {!Set<!SafetyCheckSafeBrowsingStatus>}
* @private
*/
rowClickableStatuses: {
readOnly: true,
type: Object,
value: () => new Set([
SafetyCheckSafeBrowsingStatus.ENABLED_STANDARD,
SafetyCheckSafeBrowsingStatus.ENABLED_ENHANCED,
SafetyCheckSafeBrowsingStatus.ENABLED_STANDARD_AVAILABLE_ENHANCED,
]),
},
}, },
/** @private {?MetricsBrowserProxy} */ /** @private {?MetricsBrowserProxy} */
...@@ -120,10 +135,7 @@ Polymer({ ...@@ -120,10 +135,7 @@ Polymer({
SafetyCheckInteractions.SAFETY_CHECK_SAFE_BROWSING_MANAGE); SafetyCheckInteractions.SAFETY_CHECK_SAFE_BROWSING_MANAGE);
this.metricsBrowserProxy_.recordAction( this.metricsBrowserProxy_.recordAction(
'Settings.SafetyCheck.ManageSafeBrowsing'); 'Settings.SafetyCheck.ManageSafeBrowsing');
this.openSecurityPage_();
Router.getInstance().navigateTo(
routes.SECURITY, /* dynamicParams= */ null,
/* removeSearch= */ true);
}, },
/** /**
...@@ -140,4 +152,29 @@ Polymer({ ...@@ -140,4 +152,29 @@ Polymer({
return null; return null;
} }
}, },
/**
* @private
* @return {?boolean}
*/
isRowClickable_: function() {
return this.rowClickableStatuses.has(this.status_);
},
/** @private */
onRowClick_: function() {
if (this.isRowClickable_()) {
// TODO(crbug.com/1103015): Log action and histogram:
// SafetyCheckInteractions.SAFETY_CHECK_SAFE_BROWSING_NAVIGATE
// Settings.SafetyCheck.NavigateToSafeBrowsing
this.openSecurityPage_();
}
},
/** @private */
openSecurityPage_: function() {
Router.getInstance().navigateTo(
routes.SECURITY, /* dynamicParams= */ null,
/* removeSearch= */ true);
},
}); });
...@@ -107,6 +107,7 @@ function fireSafetyCheckChromeCleanerEvent(state) { ...@@ -107,6 +107,7 @@ function fireSafetyCheckChromeCleanerEvent(state) {
* buttonAriaLabel: (string|undefined), * buttonAriaLabel: (string|undefined),
* buttonClass: (string|undefined), * buttonClass: (string|undefined),
* managedIcon: (boolean|undefined), * managedIcon: (boolean|undefined),
* rowClickable: (boolean|undefined),
* }} destructured1 * }} destructured1
*/ */
function assertSafetyCheckChild({ function assertSafetyCheckChild({
...@@ -116,7 +117,8 @@ function assertSafetyCheckChild({ ...@@ -116,7 +117,8 @@ function assertSafetyCheckChild({
buttonLabel, buttonLabel,
buttonAriaLabel, buttonAriaLabel,
buttonClass, buttonClass,
managedIcon managedIcon,
rowClickable
}) { }) {
const safetyCheckChild = page.$$('#safetyCheckChild'); const safetyCheckChild = page.$$('#safetyCheckChild');
assertTrue(safetyCheckChild.iconStatus === iconStatus); assertTrue(safetyCheckChild.iconStatus === iconStatus);
...@@ -127,6 +129,7 @@ function assertSafetyCheckChild({ ...@@ -127,6 +129,7 @@ function assertSafetyCheckChild({
!buttonAriaLabel || safetyCheckChild.buttonAriaLabel === buttonAriaLabel); !buttonAriaLabel || safetyCheckChild.buttonAriaLabel === buttonAriaLabel);
assertTrue(!buttonClass || safetyCheckChild.buttonClass === buttonClass); assertTrue(!buttonClass || safetyCheckChild.buttonClass === buttonClass);
assertTrue(!!managedIcon === !!safetyCheckChild.managedIcon); assertTrue(!!managedIcon === !!safetyCheckChild.managedIcon);
assertTrue(!!rowClickable === !!safetyCheckChild.rowClickable);
} }
/** @implements {SafetyCheckBrowserProxy} */ /** @implements {SafetyCheckBrowserProxy} */
...@@ -370,6 +373,30 @@ suite('SafetyCheckChildTests', function() { ...@@ -370,6 +373,30 @@ suite('SafetyCheckChildTests', function() {
// Managed icon not set -> no managed icon. // Managed icon not set -> no managed icon.
assertFalse(!!page.$$('#managedIcon')); assertFalse(!!page.$$('#managedIcon'));
}); });
test('testRowClickableIndicator', function() {
page.rowClickable = true;
flush();
assertTrue(!!page.$$('#rowClickableIndicator'));
assertEquals(
'cr:arrow-right',
page.$$('#rowClickableIndicator').getAttribute('iron-icon'));
});
test('testExternalRowClickableIndicator', function() {
page.rowClickable = true;
page.external = true;
flush();
assertTrue(!!page.$$('#rowClickableIndicator'));
assertEquals(
'cr:open-in-new',
page.$$('#rowClickableIndicator').getAttribute('iron-icon'));
});
test('testNoRowClickableIndicator', function() {
// rowClickable not set -> no RowClickableIndicator.
assertFalse(!!page.$$('#rowClickableIndicator'));
});
}); });
suite('SafetyCheckUpdatesChildUiTests', function() { suite('SafetyCheckUpdatesChildUiTests', function() {
...@@ -517,6 +544,7 @@ suite('SafetyCheckPasswordsChildUiTests', function() { ...@@ -517,6 +544,7 @@ suite('SafetyCheckPasswordsChildUiTests', function() {
teardown(function() { teardown(function() {
page.remove(); page.remove();
Router.getInstance().navigateTo(routes.BASIC);
}); });
test('passwordCheckingUiTest', function() { test('passwordCheckingUiTest', function() {
...@@ -536,7 +564,14 @@ suite('SafetyCheckPasswordsChildUiTests', function() { ...@@ -536,7 +564,14 @@ suite('SafetyCheckPasswordsChildUiTests', function() {
page: page, page: page,
iconStatus: SafetyCheckIconStatus.SAFE, iconStatus: SafetyCheckIconStatus.SAFE,
label: 'Passwords', label: 'Passwords',
rowClickable: true,
}); });
// User clicks the row.
page.$$('#safetyCheckChild').click();
// Ensure the correct Settings page is shown.
assertEquals(
routes.CHECK_PASSWORDS, Router.getInstance().getCurrentRoute());
}); });
test('passwordCompromisedUiTest', async function() { test('passwordCompromisedUiTest', async function() {
...@@ -586,13 +621,20 @@ suite('SafetyCheckPasswordsChildUiTests', function() { ...@@ -586,13 +621,20 @@ suite('SafetyCheckPasswordsChildUiTests', function() {
case SafetyCheckPasswordsStatus.OFFLINE: case SafetyCheckPasswordsStatus.OFFLINE:
case SafetyCheckPasswordsStatus.NO_PASSWORDS: case SafetyCheckPasswordsStatus.NO_PASSWORDS:
case SafetyCheckPasswordsStatus.SIGNED_OUT: case SafetyCheckPasswordsStatus.SIGNED_OUT:
case SafetyCheckPasswordsStatus.FEATURE_UNAVAILABLE:
assertSafetyCheckChild({
page: page,
iconStatus: SafetyCheckIconStatus.INFO,
label: 'Passwords',
});
break;
case SafetyCheckPasswordsStatus.QUOTA_LIMIT: case SafetyCheckPasswordsStatus.QUOTA_LIMIT:
case SafetyCheckPasswordsStatus.ERROR: case SafetyCheckPasswordsStatus.ERROR:
case SafetyCheckPasswordsStatus.FEATURE_UNAVAILABLE:
assertSafetyCheckChild({ assertSafetyCheckChild({
page: page, page: page,
iconStatus: SafetyCheckIconStatus.INFO, iconStatus: SafetyCheckIconStatus.INFO,
label: 'Passwords', label: 'Passwords',
rowClickable: true,
}); });
break; break;
default: default:
...@@ -623,6 +665,7 @@ suite('SafetyCheckSafeBrowsingChildUiTests', function() { ...@@ -623,6 +665,7 @@ suite('SafetyCheckSafeBrowsingChildUiTests', function() {
teardown(function() { teardown(function() {
page.remove(); page.remove();
Router.getInstance().navigateTo(routes.BASIC);
}); });
test('safeBrowsingCheckingUiTest', function() { test('safeBrowsingCheckingUiTest', function() {
...@@ -643,7 +686,13 @@ suite('SafetyCheckSafeBrowsingChildUiTests', function() { ...@@ -643,7 +686,13 @@ suite('SafetyCheckSafeBrowsingChildUiTests', function() {
page: page, page: page,
iconStatus: SafetyCheckIconStatus.SAFE, iconStatus: SafetyCheckIconStatus.SAFE,
label: 'Safe Browsing', label: 'Safe Browsing',
rowClickable: true,
}); });
// User clicks the row.
page.$$('#safetyCheckChild').click();
// Ensure the correct Settings page is shown.
assertEquals(routes.SECURITY, Router.getInstance().getCurrentRoute());
}); });
test('safeBrowsingEnabledStandardAvailableEnhancedUiTest', function() { test('safeBrowsingEnabledStandardAvailableEnhancedUiTest', function() {
...@@ -654,6 +703,7 @@ suite('SafetyCheckSafeBrowsingChildUiTests', function() { ...@@ -654,6 +703,7 @@ suite('SafetyCheckSafeBrowsingChildUiTests', function() {
page: page, page: page,
iconStatus: SafetyCheckIconStatus.SAFE, iconStatus: SafetyCheckIconStatus.SAFE,
label: 'Safe Browsing', label: 'Safe Browsing',
rowClickable: true,
}); });
}); });
...@@ -665,6 +715,7 @@ suite('SafetyCheckSafeBrowsingChildUiTests', function() { ...@@ -665,6 +715,7 @@ suite('SafetyCheckSafeBrowsingChildUiTests', function() {
page: page, page: page,
iconStatus: SafetyCheckIconStatus.SAFE, iconStatus: SafetyCheckIconStatus.SAFE,
label: 'Safe Browsing', label: 'Safe Browsing',
rowClickable: true,
}); });
}); });
...@@ -780,10 +831,11 @@ suite('SafetyCheckExtensionsChildUiTests', function() { ...@@ -780,10 +831,11 @@ suite('SafetyCheckExtensionsChildUiTests', function() {
page: page, page: page,
iconStatus: SafetyCheckIconStatus.INFO, iconStatus: SafetyCheckIconStatus.INFO,
label: 'Extensions', label: 'Extensions',
rowClickable: true,
}); });
}); });
test('extensionsSafeUiTest', function() { test('extensionsSafeUiTest', async function() {
fireSafetyCheckExtensionsEvent( fireSafetyCheckExtensionsEvent(
SafetyCheckExtensionsStatus.NO_BLOCKLISTED_EXTENSIONS); SafetyCheckExtensionsStatus.NO_BLOCKLISTED_EXTENSIONS);
flush(); flush();
...@@ -791,7 +843,14 @@ suite('SafetyCheckExtensionsChildUiTests', function() { ...@@ -791,7 +843,14 @@ suite('SafetyCheckExtensionsChildUiTests', function() {
page: page, page: page,
iconStatus: SafetyCheckIconStatus.SAFE, iconStatus: SafetyCheckIconStatus.SAFE,
label: 'Extensions', label: 'Extensions',
rowClickable: true,
}); });
// User clicks the row.
page.$$('#safetyCheckChild').click();
// Ensure the browser proxy call is done.
const url = await openWindowProxy.whenCalled('openURL');
assertEquals('chrome://extensions', url);
}); });
test('extensionsBlocklistedOffUiTest', function() { test('extensionsBlocklistedOffUiTest', function() {
......
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