Fix #8 - Dropdown not showing initial value when list is computed #7
Conversation
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
|
Hi @ssshake . I needed to add 1e8cad4, because the solution above causes a regression in data binding - the host path is no longer able to set or receive changes. |
slawomir-brzezinski-at-interxion
changed the title
slawomir-brzezinski-at-interxion
changed the title
… list is computed Cause: When the options list is a computed, the value will get assigned before it. This loses the value, because the native select's `.value` setter will change it to '' when it can't find the option (which is always the case here, because they haven't been assigned yet). The original <select><template is=dom-repeat items=getAirportsForCity(city)></select> doesn't have the issue because templates are evaluated from leaves to roots.
…alue contains something and using like: `<select value={{val::change}}>)`
Occurs only when the options list is a computed, the value will get assigned before it, which loses the value, because the native select's .value setter will change it to '' when it can't find the option (which is always the case here, because they haven't been assigned yet).
Fixed by refactoring so that we never remove the static options in the optionsList observer (which seems right, because optionList only affects the dynamic options) and only update the visible selected option using the `.selected` property (which also takes care of #4).
…changed so that currently selected value becomes unavailable. The problem is hinted in #4 (comment) and this commit introduces this sensible default. It's a sensible default as otherwise user will be able to submit a value that according to UI logic is clearly not a valid one. The native select's `.value` accessor also resets to `''` when the value is not found, so is the natural choice. This can be disabled by `keep-value-with-no-matching-option=true`
…onger able to set or receive changes
…select not receiving changes from the host path - needed to override the value property which also entailed a need to add artificial firing of the 'change' event :(
|
I had this kind of issue on Safari where the drop down list was showing no selected value. |
|
Thanks @ssshake |
|
Thanks @ssshake. How about creating a release, so users have something to hook onto that isn't a hash? |
slawomir-brzezinski-at-interxion
deleted the
fix-sometimes-not-showing-initial-value
branch
slawomir-brzezinski-at-interxion
restored the
fix-sometimes-not-showing-initial-value
branch
|
That would be a good idea to add a new version tag yes, for bower for example. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Hi @ssshake. Just found this issue in our project (see title).
See reproduction.
Cause: When the options list is a computed, the value binding will get assigned before the options computation adds any options. This loses that value, because the native select's
.valuesetter will change it to''when it can't find the option.The original doesn't have the issue because templates are evaluated from leaves to roots.
See commit message of 'the fix commit' for details.
I've also added a solution to the remark I saw in #4, one that states that the PR 'Replaces a larger bug with a smaller one':
See commit message of the second small commit for details.