Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly set the dropdown value when dealing with remote loaded choices #2573

Merged
merged 1 commit into from
Jul 10, 2015
Merged

Properly set the dropdown value when dealing with remote loaded choices #2573

merged 1 commit into from
Jul 10, 2015

Conversation

davialexandre
Copy link
Contributor

This problem happens only when we have a dropdown created from a select
that loads remote content. In this scenario, we don't know in advance
which choices will be available, so we start with a select without any
options. When an choice is selected, module.set.value will try to set
the select value, but it will fail, since there's no options.

To fix this, module.set.value will add a new option to the select, before
setting the value. This will be done only if the dropdown input is a select
and if we are dealing with remote loaded content.

This problem happens only when we have a dropdown created from a select
that loads remote content. In this scenario, we don't know in advance
which choices will be available, so we start with a select without any
options. When an choice is selected, module.set.value will try to set
the select value, but it will fail, since there's no options.

To fix this, module.set.value will add a new option to the select, before
setting the value. This will be done only if the dropdown input is a select
and if we are dealing with remote loaded content.
@jlukic jlukic added this to the 2.0.3 milestone Jul 10, 2015
jlukic added a commit that referenced this pull request Jul 10, 2015
Properly set the dropdown value when dealing with remote loaded choices
@jlukic jlukic merged commit f17f11e into Semantic-Org:next Jul 10, 2015
@jlukic
Copy link
Member

jlukic commented Jul 10, 2015

On review it seems it should do if either allowAdditions or apiSettings is used.

jlukic added a commit that referenced this pull request Jul 10, 2015
@davialexandre
Copy link
Contributor Author

It makes sense. The only problem I see is that e61b39c ended up duplicating code. Both module.set.value and module.add.value now have this:

if($input.is('select')) {
  if(settings.allowAdditions || settings.apiSettings) {
    module.debug('Adding value to select', addedValue, newValue, $input);
    module.add.optionValue(addedValue);
    }
}

What about moving this check to module.set.selected, inside the $selectedItem.each callback? I think it is a good way to remove this duplication.

jlukic added a commit that referenced this pull request Jul 20, 2015
…selected. Consolidated checks into has conditions
@jlukic
Copy link
Member

jlukic commented Jul 20, 2015

  • Added can extend select, has select input
  • Dropdown now removes option values if a user/api selection is removed
  • Added error conditions for multiple <select> initialized with multiple prop (caused me lots of headaches)

@davialexandre davialexandre deleted the fix_remote_select_dropdown branch July 28, 2015 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants