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

Fix for duplicate "None" default values - Related to BZ1428133 #633

Merged
merged 1 commit into from Mar 9, 2017

Conversation

eclarizio
Copy link
Member

@eclarizio eclarizio commented Mar 8, 2017

This change removes the need to prepend 'none' option to values as it is done in model now. It's not a strictly necessary counterpart for ManageIQ/manageiq#14240, but without it, you would see double "None"/"Choose" values when editing the dialog field.

Related to https://bugzilla.redhat.com/show_bug.cgi?id=1428133.

/cc @gmcculloug

@miq-bot add_label bug, euwe/yes

@eclarizio
Copy link
Member Author

With the latest update, without this change, ManageIQ/manageiq#14240 would produce some oddities in the UI such as potential double "None"/"Choose" values, radio buttons having a "None" option when the user could simply just not pick something, and the sample fields would have doubles as well.

Note that everything should still work fine without the UI changes, but it might be very confusing for the user seeing multiple blank values that do the same thing.

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

Continue to prepend 'none' option for radio buttons while editing the
field, because it should still be a default value option, but we do not
want to actually display a radio button with a "None" option, they can
simply leave it un-checked
@miq-bot
Copy link
Member

miq-bot commented Mar 9, 2017

Checked commit eclarizio@1962209 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 3 offenses detected

spec/helpers/application_helper/dialogs_spec.rb

@h-kataria
Copy link
Contributor

LGTM

@simaishi
Copy link
Contributor

Backported to Euwe via ManageIQ/manageiq#14259

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.

None yet

4 participants