-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Fix uibUncheckable for uibBtnRadio #6538
base: master
Are you sure you want to change the base?
Conversation
fca5338
to
ec20a33
Compare
ec20a33
to
57a4dbd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please @icfantv can you review this? I need this fix for my project.
Thanks!
@@ -324,11 +324,11 @@ describe('buttons', function() { | |||
}); | |||
|
|||
describe('uibUncheckable', function() { | |||
it('should set uncheckable', function() { | |||
it('should set disabled', function() { | |||
$scope.uncheckable = false; | |||
var btns = compileButtons('<button ng-model="model" uib-btn-radio="1">click1</button><button ng-model="model" uib-btn-radio="2" uib-uncheckable="uncheckable">click2</button>', $scope); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the tests was made with button
tags and the demo was made with label
tags?
|
||
btns.eq(1).click(); | ||
expect($scope.model).toBeNull(); | ||
expect($scope.model).toEqual(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test was wrong. If I click a disabled button the value should not change or should not be converted to null.
LGTM. Does anyone have any idea how soon or not this will be pulled into a release? This bug is impacting a project that is approaching beta launch, so I need to evaluate whether it's worth implementing a hack to work around the bug vs waiting for this fix to be merged. |
@zacronos this is my hack by the moment... gulp.task('fix:angular:bootstrap', () => {
// fix https://github.com/angular-ui/bootstrap/issues/6532
const path = './node_modules/angular-ui-bootstrap/src/buttons/';
return gulp.src(`${path}buttons.js`)
.pipe(require('gulp-replace')(
'attrs.$set(\'uncheckable\', uncheckable ? \'\' : undefined);',
'attrs.$set(\'disabled\', uncheckable ? \'disabled\' : undefined);'
))
.pipe(gulp.dest(path));
}); this should run at clean stage, before the build. |
Thanks @navarroaxel for the fix.
|
Fix #6532