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

Fixes #7723 - saving multi select field in UI component form #8246

Merged

Conversation

Zefiryn
Copy link
Contributor

@Zefiryn Zefiryn commented Jan 23, 2017

This fix resolves issue #7723 and allows to send all selected values of multi-select field in form handled by UI compontent

@vrann vrann requested a review from zetlen February 2, 2017 21:29
@@ -54,6 +54,11 @@ define([
}
break;

case 'select-multiple':
var name = item.name.substring(0,(item.name.length - 2)); //remove [] from the name ending
result[name] = Array.prototype.slice.call(item.selectedOptions).map(function(o) {return o.value;});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you are not mutating item.selectedOptions in place, there should be no need to clone it with Array.prototype.slice. Also, It might be clearer to use Underscore here, because this pattern is very common:

result[name] = _.pluck(item.selectedOptions, 'value');

@vrann vrann self-assigned this Feb 2, 2017
@vrann
Copy link
Contributor

vrann commented Feb 2, 2017

@Zefiryn thank you for contribution! Please see comments from @zetlen

@hatimeria-artur-jewula
Copy link
Contributor

@vrann Thanks for the hint. I changed the code and double checked on my end.

Copy link

@zetlen zetlen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! It looks great.

@mmansoor-magento mmansoor-magento merged commit 16170ce into magento:develop Feb 4, 2017
mmansoor-magento pushed a commit that referenced this pull request Feb 4, 2017
 - Updated composer.lock for monolog/monolog: ^1.17
@okorshenko
Copy link
Contributor

@Zefiryn Thank you for your contribution to Magento 2 project! Your pull request has been successfully merged!

@jmtakahashi
Copy link

jmtakahashi commented Feb 20, 2017

@okorshenko @Zefiryn does this also fix #8215 when using the condition "SKU is" used and adding mulitple sku's in the UI only results in the first 2 sku's being affected?

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.

7 participants