From 1cae87c299b854192249b45eaaf8dc6a6ca0d1de Mon Sep 17 00:00:00 2001 From: Jeremy Elbourn Date: Wed, 30 Sep 2015 11:30:50 -0700 Subject: [PATCH] fix(checkbox): prevent ng-click firing on didabled checkboxes. Fixes --- src/components/checkbox/checkbox.js | 32 ++- src/components/checkbox/checkbox.spec.js | 248 ++++++++++++----------- 2 files changed, 155 insertions(+), 125 deletions(-) diff --git a/src/components/checkbox/checkbox.js b/src/components/checkbox/checkbox.js index 51ddf0bb18f..1b1401c0391 100644 --- a/src/components/checkbox/checkbox.js +++ b/src/components/checkbox/checkbox.js @@ -54,7 +54,7 @@ function MdCheckboxDirective(inputDirective, $mdAria, $mdConstant, $mdTheming, $ restrict: 'E', transclude: true, require: '?ngModel', - priority:210, // Run before ngAria + priority: 210, // Run before ngAria template: '
' + '
' + @@ -73,6 +73,14 @@ function MdCheckboxDirective(inputDirective, $mdAria, $mdConstant, $mdTheming, $ tAttrs.tabindex = tAttrs.tabindex || '0'; tElement.attr('role', tAttrs.type); + // Attach a click handler in compile in order to immediately stop propagation + // (especially for ng-click) when the checkbox is disabled. + tElement.on('click', function(event) { + if (this.hasAttribute('disabled')) { + event.stopImmediatePropagation(); + } + }); + return function postLink(scope, element, attr, ngModelCtrl) { ngModelCtrl = ngModelCtrl || $mdUtil.fakeNgModel(); $mdTheming(element); @@ -83,10 +91,12 @@ function MdCheckboxDirective(inputDirective, $mdAria, $mdConstant, $mdTheming, $ ngModelCtrl.$setViewValue.bind(ngModelCtrl) ); } + $$watchExpr('ngDisabled', 'tabindex', { true: '-1', false: attr.tabindex }); + $mdAria.expectWithText(element, 'aria-label'); // Reuse the original input[type=checkbox] directive from Angular core. @@ -102,14 +112,18 @@ function MdCheckboxDirective(inputDirective, $mdAria, $mdConstant, $mdTheming, $ .on('keypress', keypressHandler) .on('mousedown', function() { scope.mouseActive = true; - $timeout(function(){ + $timeout(function() { scope.mouseActive = false; }, 100); }) .on('focus', function() { - if(scope.mouseActive === false) { element.addClass('md-focused'); } + if (scope.mouseActive === false) { + element.addClass('md-focused'); + } }) - .on('blur', function() { element.removeClass('md-focused'); }); + .on('blur', function() { + element.removeClass('md-focused'); + }); ngModelCtrl.$render = render; @@ -127,12 +141,18 @@ function MdCheckboxDirective(inputDirective, $mdAria, $mdConstant, $mdTheming, $ var keyCode = ev.which || ev.keyCode; if (keyCode === $mdConstant.KEY_CODE.SPACE || keyCode === $mdConstant.KEY_CODE.ENTER) { ev.preventDefault(); - if (!element.hasClass('md-focused')) { element.addClass('md-focused'); } + + if (!element.hasClass('md-focused')) { + element.addClass('md-focused'); + } + listener(ev); } } function listener(ev) { - if (element[0].hasAttribute('disabled')) return; + if (element[0].hasAttribute('disabled')) { + return; + } scope.$apply(function() { // Toggle the checkbox value... diff --git a/src/components/checkbox/checkbox.spec.js b/src/components/checkbox/checkbox.spec.js index 4d48e92cc35..b7268df2366 100644 --- a/src/components/checkbox/checkbox.spec.js +++ b/src/components/checkbox/checkbox.spec.js @@ -1,130 +1,143 @@ describe('mdCheckbox', function() { var CHECKED_CSS = 'md-checked'; - var $compile, $rootScope; + var $compile, $log, pageScope, $mdConstant; - beforeEach(module('ngAria')); - beforeEach(module('material.components.checkbox')); + beforeEach(module('ngAria', 'material.components.checkbox')); - beforeEach( inject(function(_$compile_, _$rootScope_){ - $compile = _$compile_; - $rootScope = _$rootScope_; + beforeEach(inject(function($injector) { + $compile = $injector.get('$compile'); + $log = $injector.get('$log'); + $mdConstant = $injector.get('$mdConstant'); + + var $rootScope = $injector.get('$rootScope'); + pageScope = $rootScope.$new(); })); - function buildInstance (template, scope){ - var element = $compile(template)(scope || $rootScope); - $rootScope.$apply(); + function compileAndLink(template, opt_scope) { + var element = $compile(template)(opt_scope || pageScope); + pageScope.$apply(); return element; } - it('should warn developers they need a label', inject(function($compile, $rootScope, $log){ + it('should warn developers they need a label', function() { spyOn($log, "warn"); - var element = buildInstance('
' + - '' + - '' + - '
'); + compileAndLink(''); expect($log.warn).toHaveBeenCalled(); - })); + }); - it('should copy text content to aria-label', inject(function($compile, $rootScope){ - var element = buildInstance('
' + - '' + - 'Some text' + - '' + - '
'); + it('should copy text content to aria-label', function() { + var element = compileAndLink( + '
' + + 'Some text' + + '
'); - var cbElements = element.find('md-checkbox'); - expect(cbElements.eq(0).attr('aria-label')).toBe('Some text'); - })); + var checkboxElement = element.find('md-checkbox').eq(0); + expect(checkboxElement.attr('aria-label')).toBe('Some text'); + }); + + it('should set checked css class and aria-checked attributes', function() { + var element = compileAndLink( + '
' + + '' + + '' + + '
'); - it('should set checked css class and aria-checked attributes', inject(function($compile, $rootScope) { - var element = buildInstance('
' + - '' + - '' + - '' + - '' + - '
'); - - $rootScope.$apply(function(){ - $rootScope.blue = false; - $rootScope.green = true; + pageScope.$apply(function() { + pageScope.blue = false; + pageScope.green = true; }); - var cbElements = element.find('md-checkbox'); + var checkboxElements = element.find('md-checkbox'); + var blueCheckbox = checkboxElements.eq(0); + var greenCheckbox = checkboxElements.eq(1); - expect(cbElements.eq(0).hasClass(CHECKED_CSS)).toEqual(false); - expect(cbElements.eq(1).hasClass(CHECKED_CSS)).toEqual(true); - expect(cbElements.eq(0).attr('aria-checked')).toEqual('false'); - expect(cbElements.eq(1).attr('aria-checked')).toEqual('true'); - expect(cbElements.eq(0).attr('role')).toEqual('checkbox'); - })); + expect(blueCheckbox.hasClass(CHECKED_CSS)).toEqual(false); + expect(greenCheckbox.hasClass(CHECKED_CSS)).toEqual(true); + + expect(blueCheckbox.attr('aria-checked')).toEqual('false'); + expect(greenCheckbox.attr('aria-checked')).toEqual('true'); - it('should be disabled with ngDisabled attr', inject(function($compile, $rootScope) { - var element = buildInstance('
' + - '' + - '' + - '
'); + expect(blueCheckbox.attr('role')).toEqual('checkbox'); + }); + + it('should be disabled with ngDisabled attr', function() { + var element = compileAndLink( + '
' + + '' + + '
'); var checkbox = element.find('md-checkbox'); - $rootScope.$apply('isDisabled = true'); - $rootScope.$apply('blue = false'); + pageScope.isDisabled = true; + pageScope.blue = false; + pageScope.$apply(); checkbox.triggerHandler('click'); - expect($rootScope.blue).toBe(false); + expect(pageScope.blue).toBe(false); - $rootScope.$apply('isDisabled = false'); + pageScope.isDisabled = false; + pageScope.$apply(); checkbox.triggerHandler('click'); - expect($rootScope.blue).toBe(true); - })); + expect(pageScope.blue).toBe(true); + }); - it('should preserve existing tabindex', inject(function($compile, $rootScope) { - var element = buildInstance('
' + - '' + - '' + - '
'); + it('should prevent click handlers from firing when disabled', function() { + pageScope.toggle = jasmine.createSpy('toggle'); - var checkbox = element.find('md-checkbox'); - expect(checkbox.attr('tabindex')).toBe('2'); - })); + var checkbox = compileAndLink( + 'On')[0]; - it('should disable with tabindex=-1', inject(function($compile, $rootScope) { - var element = buildInstance('
' + - '' + - '' + - '
'); + checkbox.click(); - var checkbox = element.find('md-checkbox'); + expect(pageScope.toggle).not.toHaveBeenCalled(); + }); + + it('should preserve existing tabindex', function() { + var element = compileAndLink( + ''); + + expect(element.attr('tabindex')).toBe('2'); + }); + + it('should disable with tabindex="-1" ', function() { + var checkbox = compileAndLink( + ''); + + pageScope.isDisabled = true; + pageScope.$apply(); - $rootScope.$apply('isDisabled = true'); expect(checkbox.attr('tabindex')).toBe('-1'); - $rootScope.$apply('isDisabled = false'); + pageScope.isDisabled = false; + pageScope.$apply(); expect(checkbox.attr('tabindex')).toBe('0'); - })); + }); - it('should not set focus state on mousedown', inject(function($compile, $rootScope) { - var checkbox = buildInstance('',$rootScope.$new()); + it('should not set focus state on mousedown', function() { + var checkbox = compileAndLink( + ''); checkbox.triggerHandler('mousedown'); expect(checkbox[0]).not.toHaveClass('md-focused'); - })); + }); - it('should set focus state on focus and remove on blur', inject(function($compile, $rootScope) { - var checkbox = buildInstance('',$rootScope.$new()); + it('should set focus state on focus and remove on blur', function() { + var checkbox = compileAndLink(''); checkbox.triggerHandler('focus'); expect(checkbox[0]).toHaveClass('md-focused'); + checkbox.triggerHandler('blur'); expect(checkbox[0]).not.toHaveClass('md-focused'); - })); + }); - it('should set focus state on keyboard interaction after clicking', inject(function($compile, $rootScope, $mdConstant) { - var checkbox = buildInstance('',$rootScope.$new()); + it('should set focus state on keyboard interaction after clicking', function() { + var checkbox = compileAndLink(''); checkbox.triggerHandler('mousedown'); checkbox.triggerHandler({ @@ -132,90 +145,87 @@ describe('mdCheckbox', function() { keyCode: $mdConstant.KEY_CODE.SPACE }); expect(checkbox[0]).toHaveClass('md-focused'); - })); + }); describe('ng core checkbox tests', function() { - function isChecked(cbEl) { - return cbEl.hasClass(CHECKED_CSS); + function isChecked(checkboxElement) { + return checkboxElement.hasClass(CHECKED_CSS); } it('should format booleans', function() { - var inputElm = buildInstance(''); + var inputElement = compileAndLink(''); - $rootScope.$apply("name = false"); - expect(isChecked(inputElm)).toBe(false); + pageScope.name = false; + pageScope.$apply(); + expect(isChecked(inputElement)).toBe(false); - $rootScope.$apply("name = true"); - expect(isChecked(inputElm)).toBe(true); + pageScope.name = true; + pageScope.$apply(); + expect(isChecked(inputElement)).toBe(true); }); - it('should support type="checkbox" with non-standard capitalization', function() { - var inputElm = buildInstance(''); + var inputElm = compileAndLink(''); inputElm.triggerHandler('click'); - expect($rootScope.checkbox).toBe(true); + expect(pageScope.checkbox).toBe(true); inputElm.triggerHandler('click'); - expect($rootScope.checkbox).toBe(false); + expect(pageScope.checkbox).toBe(false); }); - it('should allow custom enumeration', function() { - var inputElm = buildInstance(''); + var checkbox = compileAndLink( + ''); - $rootScope.$apply("name = 'y'"); - expect(isChecked(inputElm)).toBe(true); + pageScope.name = 'y'; + pageScope.$apply(); + expect(isChecked(checkbox)).toBe(true); - $rootScope.$apply("name = 'n'"); - expect(isChecked(inputElm)).toBe(false); + pageScope.name ='n'; + pageScope.$apply(); + expect(isChecked(checkbox)).toBe(false); - $rootScope.$apply("name = 'something else'"); - expect(isChecked(inputElm)).toBe(false); + pageScope.name = 'something else'; + pageScope.$apply(); + expect(isChecked(checkbox)).toBe(false); - inputElm.triggerHandler('click'); - expect($rootScope.name).toEqual('y'); + checkbox.triggerHandler('click'); + expect(pageScope.name).toEqual('y'); - inputElm.triggerHandler('click'); - expect($rootScope.name).toEqual('n'); + checkbox.triggerHandler('click'); + expect(pageScope.name).toEqual('n'); }); - it('should throw if ngTrueValue is present and not a constant expression', function() { expect(function() { - buildInstance(''); + compileAndLink(''); }).toThrow(); }); - it('should throw if ngFalseValue is present and not a constant expression', function() { expect(function() { - buildInstance(''); + compileAndLink(''); }).toThrow(); }); - it('should not throw if ngTrueValue or ngFalseValue are not present', function() { expect(function() { - buildInstance(''); + compileAndLink(''); }).not.toThrow(); }); - it('should be required if false', function() { - var inputElm = buildInstance(''); + var checkbox = compileAndLink(''); - inputElm.triggerHandler('click'); - expect(isChecked(inputElm)).toBe(true); - expect(inputElm.hasClass('ng-valid')).toBe(true); + checkbox.triggerHandler('click'); + expect(isChecked(checkbox)).toBe(true); + expect(checkbox.hasClass('ng-valid')).toBe(true); - inputElm.triggerHandler('click'); - expect(isChecked(inputElm)).toBe(false); - expect(inputElm.hasClass('ng-invalid')).toBe(true); + checkbox.triggerHandler('click'); + expect(isChecked(checkbox)).toBe(false); + expect(checkbox.hasClass('ng-invalid')).toBe(true); }); - }); - });