feature: Implement multiple option for select field #3572
Conversation
|
Code Climate has analyzed commit 4931ab8 and detected 0 issues on this pull request. View more on Code Climate. |
There was a problem hiding this comment.
Hi @zhephyn
Thanks for this contribution. It's going on the right direction!
I left some comments on the code.
I also noticed that when multiple is true the show and index views are not showing the value as expected.
Let's address the code comments and the displaying issue before the next review.
Let me know if I can help.
Thank you!
| include_blank: @field.include_blank, | ||
| multiple: @field.multiple, | ||
| aria: { | ||
| placeholder: @field.placeholder | ||
| }, | ||
| include_hidden: false |
There was a problem hiding this comment.
Is it necessary to put the aria inside the same hash as include_blank? Why aren't we keeping the same syntax as before?
I think include_hidden: false is causing an issue:
- Create a product with 2 random sizes
- Verify that sizes are correctly assigned
- Edit that product
- Deselect all the sizes
- Save
- Notice that product is keeping the 2 sizes instead empty
There was a problem hiding this comment.
The purpose of include_hidden: false was because, without it, when I select the sizes when creating a new product, an empty string is added as part of the submitted array. For example, if I was to select "large" and "small" as the sizes for a new product, on saving the product to the database, the array looks like this ["/","large","small"] without include_hidden: false. Should I remove it altogether or should I solve the error so that it can be kept? All in all, it's purpose was preventing the empty string from being saved as part of the sizes array.
There was a problem hiding this comment.
I'm not sure if we should remove or keep it, but this is the expected behavior:
If large and small is selected than the array should look like: ["large", "small"]
After having it saved as ["large", "small"] and on edit both are deselected the array should look like: []
| include_blank: @field.include_blank, | ||
| multiple: @field.multiple, | ||
| aria: { | ||
| placeholder: @field.placeholder | ||
| }, | ||
| include_hidden: false |
There was a problem hiding this comment.
| include_blank: @field.include_blank, | |
| multiple: @field.multiple, | |
| aria: { | |
| placeholder: @field.placeholder | |
| }, | |
| include_hidden: false | |
| include_blank: @field.include_blank, | |
| multiple: @field.multiple | |
| }, | |
| aria: { | |
| placeholder: @field.placeholder |
Co-authored-by: Paul Bob <[email protected]>
Co-authored-by: Paul Bob <[email protected]>
Co-authored-by: Paul Bob <[email protected]>
Description
Add a
multipleboolean option to the select field, allowing for multiple selections.Fixes #3426
Checklist:
Manual review steps
Added two extra tests(one for selecting multiple sizes when creating a new product, another for editing the sizes list of an existing product) to the select_field_spec.rb file. These all pass when i run the command "bin/test ./spec/feature/avo/select_field_spec.rb".