DialogFieldTagControl - don't add <None> for multiselects #19696
Conversation
when selecting one option from a list, having <None> (or <Choose>) in the list makes sense when selecting 0 or more options from a multiselect, neither <None> or <Choose> can be in the actual list of options. This matches what #19148 did for DialogTagDropDownList, (including introducing the `multiselect?` method). (Previously, the options were added to tags in #14348)
|
Checked commits https://github.com/himdel/manageiq/compare/cef975d90c1861c1ad9bade5a6261ac760e0e759~...20f9bed49ac65bc6558c9f07fe27fde3d1000a0f with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 |
| def multiselect? | ||
| force_multi_value | ||
| end |
There was a problem hiding this comment.
Should we add this method or just use force_multi_value on line 71? Or, if we do want multiselect?, should we just change it to an alias?
Just seems like we're making a method for the name, since it does nothing other than call the other method that already exists.
We could alternatively just call single_value? on line 71 and reverse the ternary. I think I like this idea the best.
There was a problem hiding this comment.
I'm adding it for consistency with dropdowns, which are already using the method.
But, I can change both :).
But, then it can't be single_value? which doesn't exist for dropdowns, and to me force_multi sounds misleading, we don't care about "forced" but about what we're actually using, that's why I introduced multiselect? in the first place.
(And in fact, with tagging, force_multi_value now gives the wrong value when the category doesn't allow multiselect, but the editor didn't know that, but I'm not sure what that change should affect yet.)
WDYT, anything we can use in both?
There was a problem hiding this comment.
Ah ok that makes sense, maybe we just do alias_method :multiselect?, :force_multi_value then?
|
We need to backport this to ivanchuk for https://bugzilla.redhat.com/show_bug.cgi?id=1834219 @miq-bot add_label ivanchuk/yes |
ghost
mentioned this pull request
DialogFieldTagControl - don't add <None> for multiselects (cherry picked from commit 9ce9b3d) https://bugzilla.redhat.com/show_bug.cgi?id=1834219
|
Ivanchuk backport details: |
when selecting one option from a list, having
<None>(or<Choose>) in the list makes sensewhen selecting 0 or more options from a multiselect, neither
<None>or<Choose>can be in the actual list of options.This matches what #19148 did for
DialogTagDropDownList,(including introducing the
multiselect?method).(Previously, the options were added to tags in #14348)
Cc @eclarizio - can you review please?
Cc @skateman,
Fixes ManageIQ/manageiq-ui-classic#6393
and partly addresses the issue in ManageIQ/manageiq-ui-classic#6392
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1834219
@miq-bot add_label bug, automate