Skip to content

Commit

Permalink
Address reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
ZitaNemeckova committed Mar 1, 2017
1 parent fa7828e commit 754d023
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 37 deletions.
14 changes: 8 additions & 6 deletions app/assets/javascripts/components/miq-button.js
Expand Up @@ -7,8 +7,9 @@ ManageIQ.angular.app.component('miqButton', {
primary: '<?',
onClick: '&',
},
controllerAs: 'vm',
controller: function() {
this.buttonClicked = function($event) {
this.buttonClicked = function() {
if (this.enabled) {
this.onClick();
}
Expand All @@ -30,11 +31,12 @@ ManageIQ.angular.app.component('miqButton', {
},
template: [
'<button',
'ng-class="{btn: true, \'btn-primary\': $ctrl.primary, \'btn-default\': !$ctrl.primary, disabled: !$ctrl.enabled}"',
'ng-click="$ctrl.buttonClicked()"',
'ng-attr-title="{{$ctrl.title}}"',
'ng-attr-alt="{{$ctrl.title}}">',
'{{$ctrl.name}}',
'controllerAs="vm"',

This comment has been minimized.

Copy link
@himdel

himdel Mar 17, 2017

Contributor

controllerAs="vm" ... why on a button? There's no ng-controller there, is there? (Guessing this was added before you figured out line 10 :))

This comment has been minimized.

Copy link
@AparnaKarve

AparnaKarve Mar 17, 2017

Contributor

controllerAs="vm" added twice? here and above

'ng-class="{btn: true, \'btn-primary\': vm.primary, \'btn-default\': !vm.primary, disabled: !vm.enabled}"',
'ng-click="vm.buttonClicked()"',
'ng-attr-title="{{vm.title}}"',
'ng-attr-alt="{{vm.title}}">',
'{{vm.name}}',

This comment has been minimized.

Copy link
@cben

cben Mar 14, 2017

Contributor

I don't think this is working, when creating new GCE cloud provider, the button simply says {{vm.name}}.

manageiq cloud providers

This comment has been minimized.

Copy link
@ZitaNemeckova

ZitaNemeckova Mar 14, 2017

Author Contributor

@cben Thanks. I will look into it asap.

This comment has been minimized.

Copy link
@ZitaNemeckova

ZitaNemeckova Mar 17, 2017

Author Contributor
'</button>'
].join("\n")
});
8 changes: 0 additions & 8 deletions app/assets/javascripts/directives/form_buttons.js

This file was deleted.

Expand Up @@ -3,7 +3,7 @@
- confirm ||= ""
- ng_show ||= true

%div{"ng-show" => ng_show}
%div{"ng-show" => ng_show, "ng-controller" => "buttonGroupController"}
%miq-button{:name => _("Submit"),
"disabled-title" => submit_title_off,
"enabled-title" => submit_title_on,
Expand Down
26 changes: 13 additions & 13 deletions app/views/layouts/angular/_x_custom_form_buttons_angular.html.haml
@@ -1,18 +1,18 @@
.clearfix
.pull-right.button-group{"ng-controller" => "buttonGroupController"}
%miq-button{:name => button_label,
:title => button_label,
:alt => button_label,
:enabled => "saveable(angularForm)",
'on-click' => button_click,
:primary => 'true'}
:title => button_label,
:alt => button_label,
:enabled => "saveable(angularForm)",
'on-click' => button_click,
:primary => 'true'}
%miq-button{:name => t = _("Reset"),
:title => t,
:alt => t,
:enabled => '!angularForm.$pristine',
'ng-show' => '!newRecord',
'on-click' => "resetClicked()"}
:title => t,
:alt => t,
:enabled => '!angularForm.$pristine',
'ng-show' => '!newRecord',
'on-click' => "resetClicked()"}
%miq-button{:name => t = _("Cancel"),
:title => t,
:alt => t,
'on-click' => "cancelClicked($event)"}
:title => t,
:alt => t,
'on-click' => "cancelClicked($event)"}
Expand Up @@ -11,7 +11,7 @@
:title => t,
:alt => t,
'ng-show' => '!newRecord',
:enabled => "miqService.saveable(angularForm)",
:enabled => "saveable(angularForm)",
'on-click' => "saveClicked({target: '.edit_buttons'}, true)",
:primary => 'true'}
%miq-button{:name => t = _("Reset"),
Expand Down
6 changes: 0 additions & 6 deletions app/views/static/miq-button.html.haml

This file was deleted.

5 changes: 3 additions & 2 deletions spec/javascripts/components/miq-button_spec.js
Expand Up @@ -26,6 +26,7 @@ describe('miq-button', function() {
done();
});
});

it('should disable button if form is invalid', function(done) {
$scope.validForm = false;
$scope.$digest();
Expand Down Expand Up @@ -62,8 +63,7 @@ describe('miq-button', function() {
button.trigger("click");
if (item) {
expect($scope.miqButtonClicked).toHaveBeenCalled();
}
else {
} else {
expect($scope.miqButtonClicked).not.toHaveBeenCalled();
}
});
Expand All @@ -87,6 +87,7 @@ describe('miq-button', function() {
element = $compile(element)($scope);
$scope.$digest();
}));

it('should set button attributes and classes correctly', function(done) {
setTimeout(function(){
var button = element.find("miq-button > button");
Expand Down

0 comments on commit 754d023

Please sign in to comment.