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

Generic Object - fix extra vm.angularForm and nonused angularForm #3565

Merged
merged 1 commit into from Mar 13, 2018
Merged

Generic Object - fix extra vm.angularForm and nonused angularForm #3565

merged 1 commit into from Mar 13, 2018

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Mar 9, 2018

main custom button form contains the actual form, does not get angularForm via bindings, so should not use vm

custom image component should use vm, but the function does not actually take args

(noticed these while looking at network controllers, where vm.angularForm is never right)

Cc @AparnaKarve

main custom button form contains the actual form, does not get angularForm via bindings, so should not use vm

custom image component *should* use vm, but the function does not actually take args
@himdel himdel added the bug label Mar 9, 2018
@miq-bot
Copy link
Member

miq-bot commented Mar 9, 2018

Checked commit https://github.com/himdel/manageiq-ui-classic/commit/1d29a8442386f63f18936d0f6ff4a39173edaa3b with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🍪

@@ -19,7 +19,7 @@
:title => t,
:alt => t,
:enabled => "true",
'on-click' => "vm.uploadClicked(angularForm)"}
'on-click' => "vm.uploadClicked()"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't really call this a bug, since technically, nothing is broken w.r.t vm.uploadClicked right now, or nothing is getting fixed with this change.

@@ -36,7 +36,7 @@
= "<#{_('Choose')}>"
%span.help-block{"ng-show" => "angularForm.dialog_id.$error.required"}
= _("Required")
.form-group{"ng-class" => "{'has-error': vm.angularForm.display.$invalid}"}
.form-group{"ng-class" => "{'has-error': angularForm.display.$invalid}"}
Copy link
Contributor

Choose a reason for hiding this comment

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

although angularForm.display.$invalid is correct in this case, the error would never be exposed in this field which is a checkbox with Yes/No values - so technically, there is no "invalid" value here.
Anyway, good to have this change.

@AparnaKarve
Copy link
Contributor

Above changes do not impact the forms in any way ...but are good to have - LGTM

@mzazrivec mzazrivec self-assigned this Mar 13, 2018
@mzazrivec mzazrivec added this to the Sprint 82 Ending Mar 26, 2018 milestone Mar 13, 2018
@mzazrivec mzazrivec merged commit c262342 into ManageIQ:master Mar 13, 2018
@himdel himdel deleted the god-angularForm branch March 13, 2018 13:07
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