Commit 5c202a24 authored by Gavin Williams's avatar Gavin Williams Committed by Commit Bot

Prevent user from saving invalid printer configuartion input

-Add a flag to cr-searchable-dropdown so we can track whether the user's
 current text input in the dropdown matches the previously saved value.
 We use this flag to disable the printer add button if the user typed
 invalid input into either the Manufacturer or Model dropdown.

Fixed: 950887
Change-Id: I1f632b80fd8619718f13f8a5f5332ad7a62eb69e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1880181
Commit-Queue: Gavin Williams <gavinwill@chromium.org>
Reviewed-by: default avatarBailey Berro <baileyberro@chromium.org>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710888}
parent cc9715f1
...@@ -183,14 +183,16 @@ ...@@ -183,14 +183,16 @@
<cr-searchable-drop-down id="manufacturerDropdown" <cr-searchable-drop-down id="manufacturerDropdown"
items="[[manufacturerList]]" items="[[manufacturerList]]"
label="$i18n{printerManufacturer}" label="$i18n{printerManufacturer}"
value="{{activePrinter.ppdManufacturer}}"> value="{{activePrinter.ppdManufacturer}}"
invalid="{{isManufacturerInvalid_}}">
</cr-searchable-drop-down> </cr-searchable-drop-down>
</div> </div>
<div class="settings-box two-line"> <div class="settings-box two-line">
<cr-searchable-drop-down id="modelDropdown" <cr-searchable-drop-down id="modelDropdown"
items="[[modelList]]" items="[[modelList]]"
label="$i18n{printerModel}" label="$i18n{printerModel}"
value="{{activePrinter.ppdModel}}"> value="{{activePrinter.ppdModel}}"
invalid="{{isModelInvalid_}}">
</cr-searchable-drop-down> </cr-searchable-drop-down>
</div> </div>
<div id="ppdLabel" class="cr-form-field-label"> <div id="ppdLabel" class="cr-form-field-label">
...@@ -221,7 +223,10 @@ ...@@ -221,7 +223,10 @@
disabled="[[!canAddPrinter_(activePrinter.ppdManufacturer, disabled="[[!canAddPrinter_(activePrinter.ppdManufacturer,
activePrinter.ppdModel, activePrinter.ppdModel,
activePrinter.printerPPDPath, activePrinter.printerPPDPath,
addPrinterInProgress_)]]" addPrinterInProgress_,
isManufacturerInvalid_,
isModelInvalid_)]]"
on-click="addPrinter_"> on-click="addPrinter_">
$i18n{addPrinterButtonText} $i18n{addPrinterButtonText}
</cr-button> </cr-button>
......
...@@ -401,6 +401,20 @@ Polymer({ ...@@ -401,6 +401,20 @@ Polymer({
type: String, type: String,
value: '', value: '',
}, },
/**
* Indicates whether the value in the Manufacturer dropdown is a valid
* printer manufacturer. Set by manufacturerDropdown.
* @private
*/
isManufacturerInvalid_: Boolean,
/**
* Indicates whether the value in the Model dropdown is a valid printer
* model. Set by modelDropdown.
* @private
*/
isModelInvalid_: Boolean,
}, },
observers: [ observers: [
...@@ -573,7 +587,8 @@ Polymer({ ...@@ -573,7 +587,8 @@ Polymer({
canAddPrinter_: function(ppdManufacturer, ppdModel, printerPPDPath) { canAddPrinter_: function(ppdManufacturer, ppdModel, printerPPDPath) {
return !this.addPrinterInProgress_ && return !this.addPrinterInProgress_ &&
settings.printing.isPPDInfoValid( settings.printing.isPPDInfoValid(
ppdManufacturer, ppdModel, printerPPDPath); ppdManufacturer, ppdModel, printerPPDPath) &&
!this.isManufacturerInvalid_ && !this.isModelInvalid_;
}, },
}); });
......
...@@ -95,17 +95,20 @@ suite('cr-searchable-drop-down', function() { ...@@ -95,17 +95,20 @@ suite('cr-searchable-drop-down', function() {
search('c'); search('c');
assertEquals(1, getList().length); assertEquals(1, getList().length);
assertEquals('cat', getList()[0].textContent.trim()); assertEquals('cat', getList()[0].textContent.trim());
assertTrue(dropDown.invalid);
search('at'); search('at');
assertEquals(3, getList().length); assertEquals(3, getList().length);
assertEquals('cat', getList()[0].textContent.trim()); assertEquals('cat', getList()[0].textContent.trim());
assertEquals('hat', getList()[1].textContent.trim()); assertEquals('hat', getList()[1].textContent.trim());
assertEquals('rat', getList()[2].textContent.trim()); assertEquals('rat', getList()[2].textContent.trim());
assertTrue(dropDown.invalid);
search('ra'); search('ra');
assertEquals(2, getList().length); assertEquals(2, getList().length);
assertEquals('rat', getList()[0].textContent.trim()); assertEquals('rat', getList()[0].textContent.trim());
assertEquals('rake', getList()[1].textContent.trim()); assertEquals('rake', getList()[1].textContent.trim());
assertTrue(dropDown.invalid);
}); });
test('value is set on click', function() { test('value is set on click', function() {
...@@ -120,6 +123,7 @@ suite('cr-searchable-drop-down', function() { ...@@ -120,6 +123,7 @@ suite('cr-searchable-drop-down', function() {
// Make sure final value does not change while searching. // Make sure final value does not change while searching.
search('ta'); search('ta');
assertEquals('dog', dropDown.value); assertEquals('dog', dropDown.value);
assertTrue(dropDown.invalid);
}); });
// If the update-value-on-input flag is passed, final value should be whatever // If the update-value-on-input flag is passed, final value should be whatever
...@@ -137,6 +141,7 @@ suite('cr-searchable-drop-down', function() { ...@@ -137,6 +141,7 @@ suite('cr-searchable-drop-down', function() {
// Make sure final value does change while searching. // Make sure final value does change while searching.
search('ta'); search('ta');
assertEquals('ta', dropDown.value); assertEquals('ta', dropDown.value);
assertFalse(dropDown.invalid);
}); });
test('click closes dropdown', function() { test('click closes dropdown', function() {
...@@ -320,11 +325,15 @@ suite('cr-searchable-drop-down', function() { ...@@ -320,11 +325,15 @@ suite('cr-searchable-drop-down', function() {
getList()[0].click(); getList()[0].click();
assertEquals('dog', searchInput.value); assertEquals('dog', searchInput.value);
assertFalse(dropDown.invalid);
// Make sure the search box value changes back to dog // Make sure the search box value changes back to dog
search('ta'); search('ta');
assertTrue(dropDown.invalid);
blur(); blur();
assertEquals('dog', searchInput.value); assertEquals('dog', searchInput.value);
assertFalse(dropDown.invalid);
}); });
// When a user types in the dropdown but does not choose a valid option, the // When a user types in the dropdown but does not choose a valid option, the
...@@ -336,10 +345,14 @@ suite('cr-searchable-drop-down', function() { ...@@ -336,10 +345,14 @@ suite('cr-searchable-drop-down', function() {
getList()[0].click(); getList()[0].click();
assertEquals('dog', searchInput.value); assertEquals('dog', searchInput.value);
assertFalse(dropDown.invalid);
// Make sure the search box value keeps the same text // Make sure the search box value keeps the same text
search('ta'); search('ta');
assertFalse(dropDown.invalid);
blur(); blur();
assertEquals('ta', searchInput.value); assertEquals('ta', searchInput.value);
assertFalse(dropDown.invalid);
}); });
}); });
...@@ -48,6 +48,17 @@ Polymer({ ...@@ -48,6 +48,17 @@ Polymer({
placeholder: String, placeholder: String,
/**
* Used to track in real time if the |value| in cr-searchable-drop-down
* matches the value in the underlying cr-input.These values will differ
* after a user types in input that does not match a valid dropdown option.
*/
invalid: {
type: Boolean,
value: false,
notify: true,
},
/** @type {!Array<string>} */ /** @type {!Array<string>} */
items: { items: {
type: Array, type: Array,
...@@ -76,7 +87,10 @@ Polymer({ ...@@ -76,7 +87,10 @@ Polymer({
}, },
/** @private {string} */ /** @private {string} */
searchTerm_: String, searchTerm_: {
type: String,
observer: 'updateInvalid_',
},
/** @private {boolean} */ /** @private {boolean} */
dropdownRefitPending_: Boolean, dropdownRefitPending_: Boolean,
...@@ -412,6 +426,17 @@ Polymer({ ...@@ -412,6 +426,17 @@ Polymer({
onBlur_ : function () { onBlur_ : function () {
if (!this.updateValueOnInput) { if (!this.updateValueOnInput) {
this.$.search.value = this.value; this.$.search.value = this.value;
this.searchTerm_ = '';
} }
},
/**
* If |updateValueOnInput| is true then any value is allowable so always set
* |invalid| to false.
* @private
*/
updateInvalid_: function () {
this.invalid =
!this.updateValueOnInput && this.value != this.$.search.value;
} }
}); });
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