Combobox: multiple fixes. #674
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -283,11 +283,18 @@ define([ | |
| minFilterChars: 1, | ||
|
|
||
| /** | ||
| * InputNode's value. Can be used to display something at the creation time. After that, | ||
| * each user interaction will override this with the selected item label. | ||
| * @type {string} | ||
| */ | ||
| displayedValue: "", | ||
|
|
||
| // Flag used for skipping consecutive validations, if one already run. | ||
| _justValidated: false, | ||
|
|
||
| // Flag used for post initializing the widget value, if the list has not been created yet. | ||
| _processValueAfterListInit: false, | ||
|
|
||
| createdCallback: function () { | ||
| // Declarative case (list specified declaratively inside the declarative Combobox) | ||
| var list = this.querySelector("d-list"); | ||
|
|
@@ -375,7 +382,7 @@ define([ | |
| /* jshint maxcomplexity: 17 */ | ||
| refreshRendering: function (oldValues) { | ||
| var updateReadOnly = false; | ||
| if ("list" in oldValues && this.list) { | ||
| this._initList(); | ||
| } | ||
| if ("selectionMode" in oldValues) { | ||
|
|
@@ -395,11 +402,23 @@ define([ | |
| } | ||
| if ("value" in oldValues) { | ||
| if (!this._justValidated) { | ||
| if (this.list) { | ||
| // INFO: if list is already created and attached, then we can validate the `value` value | ||
| this._validateInput(false); | ||
| } else { | ||
| // INFO: otherwise we have to delay its evaluation. | ||
| this._processValueAfterListInit = true; | ||
| } | ||
| } | ||
| } | ||
| if ("_justValidated" in oldValues) { | ||
| if (this._justValidated) { | ||
| this._justValidated = false; | ||
| } | ||
| } | ||
| if ("displayedValue" in oldValues) { | ||
| this.inputNode.value = this.displayedValue; | ||
| } | ||
|
There was a problem hiding this comment. How about just saying There was a problem hiding this comment. I have already tried this solution before, but it wasn't working and I couldnt explain the reason. Do you have any hint maybe? |
||
| }, | ||
|
|
||
| /** | ||
|
|
@@ -452,31 +471,34 @@ define([ | |
| }, | ||
|
|
||
| _initList: function () { | ||
| // TODO | ||
| // This is a workaround waiting for a proper mechanism (at the level | ||
| // of delite/Store - delite/StoreMap) to allow a store-based widget | ||
| // to delegate the store-related functions to a parent widget (delite#323). | ||
| if (!this.list.attached) { | ||
| this.list.attachedCallback(); | ||
| } | ||
|
|
||
| // Class added on the list such that Combobox' theme can have a specific | ||
| // CSS selector for elements inside the List when used as dropdown in | ||
| // the combo. | ||
| $(this.list).addClass("d-combobox-list"); | ||
|
|
||
| // The drop-down is hidden initially | ||
| $(this.list).addClass("d-hidden"); | ||
|
|
||
| // The role=listbox is required for the list part of a combobox by the | ||
| // aria spec of role=combobox | ||
| this.list.type = "listbox"; | ||
|
|
||
| this.list.selectionMode = this.selectionMode === "single" ? | ||
| "radio" : "multiple"; | ||
|
|
||
| this._initHandlers(); | ||
|
|
||
| if (this._processValueAfterListInit) { | ||
| this.notifyCurrentValue("value"); | ||
| delete this._processValueAfterListInit; | ||
| } | ||
| }, | ||
|
|
||
|
|
@@ -684,7 +706,7 @@ define([ | |
| // save what user typed at each keystroke. | ||
| this.value = inputElement.value; | ||
| if (this._useCenteredDropDown()) { | ||
| this.displayedValue = inputElement.value; | ||
|
There was a problem hiding this comment. I filed #688 for the remaining task to keep displayedValue in sync w/the actual displayed value. |
||
| } | ||
| this.handleOnInput(this.value); // emit "input" event. | ||
|
|
||
|
|
@@ -750,28 +772,30 @@ define([ | |
| }.bind(this), inputElement); | ||
| }, | ||
|
|
||
| _validateInput: function (userInteraction) { | ||
| if (this.selectionMode === "single") { | ||
| this._validateSingle(userInteraction); | ||
| } else { | ||
| this._validateMultiple(userInteraction); | ||
| } | ||
| this._justValidated = true; | ||
| this.notifyCurrentValue("_justValidated"); | ||
| }, | ||
|
|
||
| _validateSingle: function (userInteraction) { | ||
| if (userInteraction) { | ||
| var selectedItem = this.list.selectedItem; | ||
| // selectedItem non-null because List in radio selection mode, but | ||
| // the List can be empty, so: | ||
| this.displayedValue = selectedItem ? this._getItemLabel(selectedItem) : ""; | ||
| this.value = selectedItem ? this._getItemValue(selectedItem) : ""; | ||
| } else { | ||
| var item = this._retrieveItemFromSource(this.value); | ||
| this.displayedValue = item ? item[this.list.labelAttr || this.list.labelFunc] : this.value; | ||
| } | ||
| }, | ||
|
|
||
| _validateMultiple: function (userInteraction) { | ||
| var n; | ||
| if (userInteraction) { | ||
| var selectedItems = this.list.selectedItems; | ||
|
|
@@ -795,7 +819,7 @@ define([ | |
| // make sure this is already done when FormValueWidget.handleOnInput() runs. | ||
| this.valueNode.value = value; | ||
| this.handleOnInput(this.value); // emit "input" event | ||
| } else { | ||
| var items = []; | ||
| if (typeof this.value === "string") { | ||
| if (this.value.length > 0) { | ||
|
|
@@ -809,11 +833,12 @@ define([ | |
| } // else empty array. No pre-set values. | ||
| n = items.length; | ||
| if (n > 1) { | ||
| this.displayedValue = string.substitute(this.multipleChoiceMsg, {items: n}); | ||
| } else if (n === 1) { | ||
| var item = this._retrieveItemFromSource(items[0]); | ||
| this.displayedValue = item ? item[this.list.labelAttr || this.list.labelFunc] : this.value; | ||
| } else { | ||
| this.displayedValue = this.multipleChoiceNoSelectionMsg; | ||
| } | ||
| } | ||
| }, | ||
|
|
@@ -848,6 +873,21 @@ define([ | |
| this.openDropDown(); | ||
| }, | ||
|
|
||
| _retrieveItemFromSource: function (key) { | ||
| var item = null, | ||
| _source = this.source || (this.list && this.list.source); | ||
| if (_source) { | ||
| if (Array.isArray(_source)) { | ||
| item = _source.filter(function (i) { | ||
| return i[this.list.valueAttr || this.list.labelAttr] === key; | ||
| }.bind(this))[0]; | ||
| } else { | ||
| item = _source.getSync && _source.getSync(key); | ||
| } | ||
| } | ||
| return item; | ||
| }, | ||
|
There was a problem hiding this comment. It's strange to have code that only works with a sync store, especially considering that you are working on auto-complete which is asynchronous. There was a problem hiding this comment. I think I can make this part specific for only when in not auto-complete mode (hasDownArrow=false). Do you agree? |
||
|
|
||
| /** | ||
| * Sets the new list's query. | ||
| * This method can be overridden when using other store types. | ||
|
|
@@ -960,8 +1000,10 @@ define([ | |
| _dropDownKeyUpHandler: dcl.superCall(function (sup) { | ||
| return function () { | ||
| if (this.hasDownArrow) { | ||
| if (this.inputNode.value.length === 0) { | ||
| if (this.minFilterChars === 0) { | ||
| this.list.query = this._getDefaultQuery(); | ||
| } | ||
|
There was a problem hiding this comment. I guess this is the code change to "Restore down-arrow key original behaviour, even if the inputNode is empty."? It doesn't seem quite right. For example:
There was a problem hiding this comment.
|
||
| } else if (this.inputNode.value.length < this.minFilterChars) { | ||
| return; | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is confusing to me, why don't you just do:
all the time, without the
if()statements?