Commit 7cd1a012 authored by rbpotter's avatar rbpotter Committed by Commit Bot

WebUI Polymer2 Migration: Fix pref indicator inheritance issue

Overriding Polymer properties defined in a behavior was never fully
specified, and could lead to weird corner cases, which Polymer 2
explicitly decided to address. One new specification is that
properties declared as computed in a behavior should not be overridden
in elements implementing the behavior.

As a result the indicatorTooltip property in CrPolicyIndicatorBehavior
can no longer be overridden in Polymer2. Remove this property from
the behavior and put it in the 3 elements implementing the behavior
individually, while retaining the behavior's computation method since
it is used by all 3.

This causes
CrElementsPolicyNetworkIndicatorTest.All (CrOS only) and
CrElementsPolicyPrefIndicatorTest.All
to pass with Polymer 2.

Bug: 738611
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Iab7824c5d69c59b00fa0f1a173e3d5c85ec7d2e4
Reviewed-on: https://chromium-review.googlesource.com/1135631
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: default avatarDemetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575155}
parent 32b19c70
......@@ -29,7 +29,10 @@ suite('CrPolicyIndicatorBehavior', function() {
assertTrue(indicator.indicatorVisible);
assertEquals('cr20:domain', indicator.indicatorIcon);
assertEquals('policy', indicator.indicatorTooltip);
assertEquals(
'policy',
indicator.getIndicatorTooltip(
indicator.indicatorType, indicator.indicatorSourceName));
});
test('recommended indicator', function() {
......@@ -53,7 +56,10 @@ suite('CrPolicyIndicatorBehavior', function() {
assertTrue(indicator.indicatorVisible);
assertEquals('cr:extension', indicator.indicatorIcon);
assertEquals('extension: Extension name', indicator.indicatorTooltip);
assertEquals(
'extension: Extension name',
indicator.getIndicatorTooltip(
indicator.indicatorType, indicator.indicatorSourceName));
});
test('extension indicator without extension name', function() {
......@@ -62,7 +68,10 @@ suite('CrPolicyIndicatorBehavior', function() {
assertTrue(indicator.indicatorVisible);
assertEquals('cr:extension', indicator.indicatorIcon);
assertEquals('extension', indicator.indicatorTooltip);
assertEquals(
'extension',
indicator.getIndicatorTooltip(
indicator.indicatorType, indicator.indicatorSourceName));
});
if (cr.isChromeOS) {
......@@ -72,7 +81,10 @@ suite('CrPolicyIndicatorBehavior', function() {
assertTrue(indicator.indicatorVisible);
assertEquals('cr:group', indicator.indicatorIcon);
assertEquals('shared: user@example.com', indicator.indicatorTooltip);
assertEquals(
'shared: user@example.com',
indicator.getIndicatorTooltip(
indicator.indicatorType, indicator.indicatorSourceName));
});
}
});
......@@ -8,7 +8,7 @@
<template>
<style include="cr-hidden-style"></style>
<cr-tooltip-icon hidden$="[[!indicatorVisible]]"
tooltip-text="[[indicatorTooltip]]" icon-class="[[indicatorIcon]]"
tooltip-text="[[indicatorTooltip_]]" icon-class="[[indicatorIcon]]"
icon-aria-label="[[iconAriaLabel]]">
</cr-tooltip-icon>
</template>
......
......@@ -10,5 +10,21 @@ Polymer({
properties: {
iconAriaLabel: String,
/** @private {string} */
indicatorTooltip_: {
type: String,
computed: 'getIndicatorTooltip_(indicatorType, indicatorSourceName)',
},
},
/**
* @param {!CrPolicyIndicatorType} type
* @param {string} name The name associated with the indicator. See
* chrome.settingsPrivate.PrefObject.controlledByName
* @return {string} The tooltip text for |type|.
*/
getIndicatorTooltip_: function(indicatorType, indicatorSourceName) {
return this.getIndicatorTooltip(indicatorType, indicatorSourceName);
},
});
......@@ -68,11 +68,6 @@ var CrPolicyIndicatorBehavior = {
type: String,
computed: 'getIndicatorIcon_(indicatorType)',
},
indicatorTooltip: {
type: String,
computed: 'getIndicatorTooltip(indicatorType, indicatorSourceName)',
},
},
/**
......
......@@ -9,7 +9,7 @@
<template>
<style include="cr-hidden-style"></style>
<cr-tooltip-icon hidden$="[[!indicatorVisible]]"
tooltip-text="[[indicatorTooltip]]" icon-class="[[indicatorIcon]]">
tooltip-text="[[indicatorTooltip_]]" icon-class="[[indicatorIcon]]">
</cr-tooltip-icon>
</template>
<script src="cr_policy_network_indicator.js"></script>
......
......@@ -25,8 +25,8 @@ Polymer({
*/
recommended_: Object,
/** @override */
indicatorTooltip: {
/** @private */
indicatorTooltip_: {
type: String,
computed: 'getNetworkIndicatorTooltip_(indicatorType, property.*)',
},
......@@ -75,6 +75,9 @@ Polymer({
* @private
*/
getNetworkIndicatorTooltip_: function() {
if (this.property === undefined)
return '';
var matches;
if (this.indicatorType == CrPolicyIndicatorType.RECOMMENDED &&
this.property) {
......
......@@ -8,7 +8,7 @@
<template>
<style include="cr-hidden-style"></style>
<cr-tooltip-icon hidden$="[[!indicatorVisible]]"
tooltip-text="[[indicatorTooltip]]" icon-class="[[indicatorIcon]]"
tooltip-text="[[indicatorTooltip_]]" icon-class="[[indicatorIcon]]"
icon-aria-label="[[iconAriaLabel]]">
</cr-tooltip-icon>
</template>
......
......@@ -25,8 +25,8 @@ Polymer({
computed: 'getIndicatorTypeForPref_(pref.controlledBy, pref.enforcement)',
},
/** @override */
indicatorTooltip: {
/** @private */
indicatorTooltip_: {
type: String,
computed: 'getIndicatorTooltipForPref_(indicatorType, pref.*)',
},
......@@ -71,6 +71,9 @@ Polymer({
* @private
*/
getIndicatorTooltipForPref_: function(indicatorType) {
if (this.pref === undefined)
return '';
var matches = this.pref && this.pref.value == this.pref.recommendedValue;
return this.getIndicatorTooltip(
indicatorType, this.pref.controlledByName || '', matches);
......
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