search

ManageIQ / ui-components

Angular UI Components for ManageIQ
Apache License 2.0
16 stars 52 forks source link

Don't set default value for non-applicable fields #442

Closed mzazrivec closed 4 years ago

mzazrivec commented 4 years ago
  1. create a service dialog with two radio buttons: one of the doesn't have default value set, the other one does
  2. create a catalog item that uses the above service dialog
  3. try to order the service

Without this fix, the field with no default value would always default to the first choice. Screenshot from 2020-06-17 13-41-18

With this fix in place, the no default value is being honored. Screenshot from 2020-06-17 13-12-46

https://bugzilla.redhat.com/show_bug.cgi?id=1713212

miq-bot commented 4 years ago

Checked commit https://github.com/mzazrivec/ui-components/commit/a84d2c733c1d3dd9ff00614f09fc6bf01e3d8967 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint 0 files checked, 0 offenses detected Everything looks fine. :+1:

himdel commented 4 years ago

This will also cause selects to have no default value by default. Is that intended?

(I believe "selects default to the first option" was a feature at some point, but I'm OK to see it go away.)

mzazrivec commented 4 years ago

@himdel This is what dropdown would look like with my change Screenshot from 2020-06-18 13-56-49

Dropdown without a default value would render <None> as a default choice, dropdown with a default value would render the default value. That I believe is the correct behavior here. I studied the code a bit, but wasn't able to coprehend why would we want to always set a default value.

himdel commented 4 years ago

Yes, I agree, as long as that's intentional.. :)

The "preselect the first option if none selected" logic never made much sense to me either.

simaishi commented 4 years ago

Ivanchuk backport details:

$ git log -1
commit ed77da4b7e0d24236a98b9b5ba8c27cd0a206411
Author: Martin Hradil <mhradil@redhat.com>
Date:   Thu Jun 18 14:22:57 2020 +0000

    Merge pull request #442 from mzazrivec/dont_set_default_value_when_not_needed

    Don't set default value for non-applicable fields

    (cherry picked from commit d21e49168fa8ef7cc0702c6cfbc29d9f342e560a)

    https://bugzilla.redhat.com/show_bug.cgi?id=1713212
simaishi commented 4 years ago

Jansa backport details:

$ git log -1
commit c5c74d88996227691182562073d802a4245acd23
Author: Martin Hradil <mhradil@redhat.com>
Date:   Thu Jun 18 14:22:57 2020 +0000

    Merge pull request #442 from mzazrivec/dont_set_default_value_when_not_needed

    Don't set default value for non-applicable fields

    (cherry picked from commit d21e49168fa8ef7cc0702c6cfbc29d9f342e560a)