From 5d9b169fa9145d61f67e89a6686e49b4f914de6d Mon Sep 17 00:00:00 2001 From: Adam Bradley Date: Sun, 6 Mar 2016 20:30:54 -0600 Subject: [PATCH] fix(radio): improve group/button value comparisons --- ionic/components/radio/radio-button.ts | 16 +- ionic/components/radio/radio-group.ts | 6 +- ionic/components/radio/test/radio.spec.ts | 115 +++++++++++ ionic/util/test/util.spec.ts | 229 ++++++++++++++++++++++ ionic/util/util.ts | 18 ++ 5 files changed, 374 insertions(+), 10 deletions(-) create mode 100644 ionic/components/radio/test/radio.spec.ts diff --git a/ionic/components/radio/radio-button.ts b/ionic/components/radio/radio-button.ts index d5130f1c33..61003dd66e 100644 --- a/ionic/components/radio/radio-button.ts +++ b/ionic/components/radio/radio-button.ts @@ -1,7 +1,7 @@ import {Component, Optional, Input, Output, HostListener, EventEmitter} from 'angular2/core'; import {Form} from '../../util/form'; -import {isTrueProperty, isPresent, isBlank} from '../../util/util'; +import {isTrueProperty, isPresent, isBlank, isCheckedProperty} from '../../util/util'; import {Item} from '../item/item'; import {ListHeader} from '../list/list'; import {RadioGroup} from './radio-group'; @@ -50,7 +50,7 @@ export class RadioButton { private _checked: boolean = false; private _disabled: boolean = false; private _labelId: string; - private _value = null; + private _value: any = null; /** * @private @@ -58,9 +58,9 @@ export class RadioButton { id: string; /** - * @output {RadioButton} expression to be evaluated when selected + * @output {any} expression to be evaluated when selected */ - @Output() select: EventEmitter = new EventEmitter(); + @Output() select: EventEmitter = new EventEmitter(); constructor( private _form: Form, @@ -87,12 +87,12 @@ export class RadioButton { * @private */ @Input() - get value() { + get value(): any { // if the value is not defined then use it's unique id return isBlank(this._value) ? this.id : this._value; } - set value(val) { + set value(val: any) { this._value = val; } @@ -142,8 +142,8 @@ export class RadioButton { * @private */ ngOnInit() { - if (this._group && isPresent(this._group.value) && this._group.value == this.value) { - this.checked = true; + if (this._group && isPresent(this._group.value)) { + this.checked = isCheckedProperty(this._group.value, this.value); } } diff --git a/ionic/components/radio/radio-group.ts b/ionic/components/radio/radio-group.ts index 6dc45cabb3..8b30ca72a2 100644 --- a/ionic/components/radio/radio-group.ts +++ b/ionic/components/radio/radio-group.ts @@ -3,7 +3,7 @@ import {NG_VALUE_ACCESSOR} from 'angular2/common'; import {RadioButton} from './radio-button'; import {ListHeader} from '../list/list'; -import {isPresent} from '../../util/util'; +import {isPresent, isCheckedProperty} from '../../util/util'; const RADIO_VALUE_ACCESSOR = new Provider( NG_VALUE_ACCESSOR, {useExisting: forwardRef(() => RadioGroup), multi: true}); @@ -145,16 +145,18 @@ export class RadioGroup { */ private _update() { // loop through each of the radiobuttons + let hasChecked = false; this._btns.forEach(radioButton => { // check this radiobutton if its value is // the same as the radiogroups value - radioButton.checked = (radioButton.value == this.value); + radioButton.checked = isCheckedProperty(this.value, radioButton.value) && !hasChecked; if (radioButton.checked) { // if this button is checked, then set it as // the radiogroup's active descendant this._setActive(radioButton); + hasChecked = true; } }); } diff --git a/ionic/components/radio/test/radio.spec.ts b/ionic/components/radio/test/radio.spec.ts new file mode 100644 index 0000000000..b609c89fda --- /dev/null +++ b/ionic/components/radio/test/radio.spec.ts @@ -0,0 +1,115 @@ +import {RadioGroup, RadioButton, Form} from '../../../../ionic'; + +export function run() { + describe('RadioGroup', () => { + + describe('_update', () => { + + it('should set checked via string values', () => { + let rb1 = createRadioButton(); + rb1.value = 'string1'; + let rb2 = createRadioButton(); + rb2.value = 'string2'; + let rb3 = createRadioButton(); + rb3.value = 'string3'; + + rg.value = 'string1'; + rg._update(); + + expect(rb1.checked).toEqual(true); + expect(rb2.checked).toEqual(false); + expect(rb3.checked).toEqual(false); + }); + + it('should set checked via string group value, and number button values', () => { + let rb1 = createRadioButton(); + rb1.value = 1; + let rb2 = createRadioButton(); + rb2.value = 2; + let rb3 = createRadioButton(); + rb3.value = 3; + + rg.value = '1'; + rg._update(); + + expect(rb1.checked).toEqual(true); + expect(rb2.checked).toEqual(false); + expect(rb3.checked).toEqual(false); + }); + + it('should set checked via number group value, and string button values', () => { + let rb1 = createRadioButton(); + rb1.value = '1'; + let rb2 = createRadioButton(); + rb2.value = '2'; + let rb3 = createRadioButton(); + rb3.value = '3'; + + rg.value = 1; + rg._update(); + + expect(rb1.checked).toEqual(true); + expect(rb2.checked).toEqual(false); + expect(rb3.checked).toEqual(false); + }); + + it('should set checked via empty string group value, and one empty string button value', () => { + let rb1 = createRadioButton(); + rb1.value = ''; + let rb2 = createRadioButton(); + rb2.value = 'value2'; + let rb3 = createRadioButton(); + rb3.value = 'value3'; + + rg.value = ''; + rg._update(); + + expect(rb1.checked).toEqual(true); + expect(rb2.checked).toEqual(false); + expect(rb3.checked).toEqual(false); + }); + + it('should only check at most one value', () => { + let rb1 = createRadioButton(); + rb1.value = 'string1'; + let rb2 = createRadioButton(); + rb2.value = 'string1'; + let rb3 = createRadioButton(); + rb3.value = 'string1'; + + rg.value = 'string1'; + rg._update(); + + expect(rb1.checked).toEqual(true); + expect(rb2.checked).toEqual(false); + expect(rb3.checked).toEqual(false); + }); + + }); + + let rg: RadioGroup; + let form: Form; + + function createRadioButton() { + return new RadioButton(form, null, rg); + } + + function mockRenderer(): any { + return { + setElementAttribute: function(){} + } + } + + function mockElementRef(): any { + return { + nativeElement: document.createElement('div') + } + } + + beforeEach(() => { + rg = new RadioGroup(mockRenderer(), mockElementRef()); + form = new Form(); + }); + + }); +} diff --git a/ionic/util/test/util.spec.ts b/ionic/util/test/util.spec.ts index fe314e63cc..23c9921ff3 100644 --- a/ionic/util/test/util.spec.ts +++ b/ionic/util/test/util.spec.ts @@ -3,6 +3,235 @@ import * as util from '../../../ionic/util'; export function run() { describe('extend', function() { + describe('isCheckedProperty', function() { + + it('should test a=undefined', () => { + expect(util.isCheckedProperty(undefined, undefined)).toBe(true); + expect(util.isCheckedProperty(undefined, null)).toBe(true); + expect(util.isCheckedProperty(undefined, '')).toBe(true); + expect(util.isCheckedProperty(undefined, 'string')).toBe(false); + expect(util.isCheckedProperty(undefined, 0)).toBe(false); + expect(util.isCheckedProperty(undefined, '0')).toBe(false); + expect(util.isCheckedProperty(undefined, 1000)).toBe(false); + expect(util.isCheckedProperty(undefined, '1000')).toBe(false); + expect(util.isCheckedProperty(undefined, -1)).toBe(false); + expect(util.isCheckedProperty(undefined, true)).toBe(false); + expect(util.isCheckedProperty(undefined, false)).toBe(false); + expect(util.isCheckedProperty(undefined, 'true')).toBe(false); + expect(util.isCheckedProperty(undefined, 'false')).toBe(false); + }); + + it('should test a=null', () => { + expect(util.isCheckedProperty(null, undefined)).toBe(true); + expect(util.isCheckedProperty(null, null)).toBe(true); + expect(util.isCheckedProperty(null, '')).toBe(true); + expect(util.isCheckedProperty(null, 'string')).toBe(false); + expect(util.isCheckedProperty(null, 0)).toBe(false); + expect(util.isCheckedProperty(null, '0')).toBe(false); + expect(util.isCheckedProperty(null, 1000)).toBe(false); + expect(util.isCheckedProperty(null, '1000')).toBe(false); + expect(util.isCheckedProperty(null, -1)).toBe(false); + expect(util.isCheckedProperty(null, true)).toBe(false); + expect(util.isCheckedProperty(null, false)).toBe(false); + expect(util.isCheckedProperty(null, 'true')).toBe(false); + expect(util.isCheckedProperty(null, 'false')).toBe(false); + }); + + it('should test a=""', () => { + expect(util.isCheckedProperty('', undefined)).toBe(true); + expect(util.isCheckedProperty('', null)).toBe(true); + expect(util.isCheckedProperty('', '')).toBe(true); + expect(util.isCheckedProperty('', 'string')).toBe(false); + expect(util.isCheckedProperty('', 0)).toBe(false); + expect(util.isCheckedProperty('', '0')).toBe(false); + expect(util.isCheckedProperty('', 1000)).toBe(false); + expect(util.isCheckedProperty('', '1000')).toBe(false); + expect(util.isCheckedProperty('', -1)).toBe(false); + expect(util.isCheckedProperty('', true)).toBe(false); + expect(util.isCheckedProperty('', false)).toBe(false); + expect(util.isCheckedProperty('', 'true')).toBe(false); + expect(util.isCheckedProperty('', 'false')).toBe(false); + }); + + it('should test a=true', () => { + expect(util.isCheckedProperty(true, undefined)).toBe(false); + expect(util.isCheckedProperty(true, null)).toBe(false); + expect(util.isCheckedProperty(true, '')).toBe(false); + expect(util.isCheckedProperty(true, 'string')).toBe(false); + expect(util.isCheckedProperty(true, 0)).toBe(false); + expect(util.isCheckedProperty(true, '0')).toBe(false); + expect(util.isCheckedProperty(true, 1000)).toBe(false); + expect(util.isCheckedProperty(true, '1000')).toBe(false); + expect(util.isCheckedProperty(true, -1)).toBe(false); + expect(util.isCheckedProperty(true, true)).toBe(true); + expect(util.isCheckedProperty(true, false)).toBe(false); + expect(util.isCheckedProperty(true, 'true')).toBe(true); + expect(util.isCheckedProperty(true, 'false')).toBe(false); + }); + + it('should test a="true"', () => { + expect(util.isCheckedProperty('true', undefined)).toBe(false); + expect(util.isCheckedProperty('true', null)).toBe(false); + expect(util.isCheckedProperty('true', '')).toBe(false); + expect(util.isCheckedProperty('true', 'string')).toBe(false); + expect(util.isCheckedProperty('true', 0)).toBe(false); + expect(util.isCheckedProperty('true', '0')).toBe(false); + expect(util.isCheckedProperty('true', 1000)).toBe(false); + expect(util.isCheckedProperty('true', '1000')).toBe(false); + expect(util.isCheckedProperty('true', -1)).toBe(false); + expect(util.isCheckedProperty('true', true)).toBe(true); + expect(util.isCheckedProperty('true', false)).toBe(false); + expect(util.isCheckedProperty('true', 'true')).toBe(true); + expect(util.isCheckedProperty('true', 'false')).toBe(false); + }); + + it('should test a=false', () => { + expect(util.isCheckedProperty(false, undefined)).toBe(false); + expect(util.isCheckedProperty(false, null)).toBe(false); + expect(util.isCheckedProperty(false, '')).toBe(false); + expect(util.isCheckedProperty(false, 'string')).toBe(false); + expect(util.isCheckedProperty(false, 0)).toBe(false); + expect(util.isCheckedProperty(false, '0')).toBe(false); + expect(util.isCheckedProperty(false, 1000)).toBe(false); + expect(util.isCheckedProperty(false, '1000')).toBe(false); + expect(util.isCheckedProperty(false, -1)).toBe(false); + expect(util.isCheckedProperty(false, true)).toBe(false); + expect(util.isCheckedProperty(false, false)).toBe(true); + expect(util.isCheckedProperty(false, 'true')).toBe(false); + expect(util.isCheckedProperty(false, 'false')).toBe(true); + }); + + it('should test a="false"', () => { + expect(util.isCheckedProperty('false', undefined)).toBe(false); + expect(util.isCheckedProperty('false', null)).toBe(false); + expect(util.isCheckedProperty('false', '')).toBe(false); + expect(util.isCheckedProperty('false', 'string')).toBe(false); + expect(util.isCheckedProperty('false', 0)).toBe(false); + expect(util.isCheckedProperty('false', '0')).toBe(false); + expect(util.isCheckedProperty('false', 1000)).toBe(false); + expect(util.isCheckedProperty('false', '1000')).toBe(false); + expect(util.isCheckedProperty('false', -1)).toBe(false); + expect(util.isCheckedProperty('false', true)).toBe(false); + expect(util.isCheckedProperty('false', false)).toBe(true); + expect(util.isCheckedProperty('false', 'true')).toBe(false); + expect(util.isCheckedProperty('false', 'false')).toBe(true); + }); + + it('should test a=0', () => { + expect(util.isCheckedProperty(0, undefined)).toBe(false); + expect(util.isCheckedProperty(0, null)).toBe(false); + expect(util.isCheckedProperty(0, '')).toBe(false); + expect(util.isCheckedProperty(0, 'string')).toBe(false); + expect(util.isCheckedProperty(0, 0)).toBe(true); + expect(util.isCheckedProperty(0, '0')).toBe(true); + expect(util.isCheckedProperty(0, 1000)).toBe(false); + expect(util.isCheckedProperty(0, '1000')).toBe(false); + expect(util.isCheckedProperty(0, -1)).toBe(false); + expect(util.isCheckedProperty(0, true)).toBe(false); + expect(util.isCheckedProperty(0, false)).toBe(false); + expect(util.isCheckedProperty(0, 'true')).toBe(false); + expect(util.isCheckedProperty(0, 'false')).toBe(false); + }); + + it('should test a="0"', () => { + expect(util.isCheckedProperty('0', undefined)).toBe(false); + expect(util.isCheckedProperty('0', null)).toBe(false); + expect(util.isCheckedProperty('0', '')).toBe(false); + expect(util.isCheckedProperty('0', 'string')).toBe(false); + expect(util.isCheckedProperty('0', 0)).toBe(true); + expect(util.isCheckedProperty('0', '0')).toBe(true); + expect(util.isCheckedProperty('0', 1000)).toBe(false); + expect(util.isCheckedProperty('0', '1000')).toBe(false); + expect(util.isCheckedProperty('0', -1)).toBe(false); + expect(util.isCheckedProperty('0', true)).toBe(false); + expect(util.isCheckedProperty('0', false)).toBe(false); + expect(util.isCheckedProperty('0', 'true')).toBe(false); + expect(util.isCheckedProperty('0', 'false')).toBe(false); + }); + + it('should test a=1000', () => { + expect(util.isCheckedProperty(1000, undefined)).toBe(false); + expect(util.isCheckedProperty(1000, null)).toBe(false); + expect(util.isCheckedProperty(1000, '')).toBe(false); + expect(util.isCheckedProperty(1000, 'string')).toBe(false); + expect(util.isCheckedProperty(1000, 0)).toBe(false); + expect(util.isCheckedProperty(1000, '0')).toBe(false); + expect(util.isCheckedProperty(1000, 1000)).toBe(true); + expect(util.isCheckedProperty(1000, '1000')).toBe(true); + expect(util.isCheckedProperty(1000, -1)).toBe(false); + expect(util.isCheckedProperty(1000, true)).toBe(false); + expect(util.isCheckedProperty(1000, false)).toBe(false); + expect(util.isCheckedProperty(1000, 'true')).toBe(false); + expect(util.isCheckedProperty(1000, 'false')).toBe(false); + }); + + it('should test a="1000"', () => { + expect(util.isCheckedProperty('1000', undefined)).toBe(false); + expect(util.isCheckedProperty('1000', null)).toBe(false); + expect(util.isCheckedProperty('1000', '')).toBe(false); + expect(util.isCheckedProperty('1000', 'string')).toBe(false); + expect(util.isCheckedProperty('1000', 0)).toBe(false); + expect(util.isCheckedProperty('1000', '0')).toBe(false); + expect(util.isCheckedProperty('1000', 1000)).toBe(true); + expect(util.isCheckedProperty('1000', '1000')).toBe(true); + expect(util.isCheckedProperty('1000', -1)).toBe(false); + expect(util.isCheckedProperty('1000', true)).toBe(false); + expect(util.isCheckedProperty('1000', false)).toBe(false); + expect(util.isCheckedProperty('1000', 'true')).toBe(false); + expect(util.isCheckedProperty('1000', 'false')).toBe(false); + }); + + it('should test a=-1', () => { + expect(util.isCheckedProperty(-1, undefined)).toBe(false); + expect(util.isCheckedProperty(-1, null)).toBe(false); + expect(util.isCheckedProperty(-1, '')).toBe(false); + expect(util.isCheckedProperty(-1, 'string')).toBe(false); + expect(util.isCheckedProperty(-1, 0)).toBe(false); + expect(util.isCheckedProperty(-1, '0')).toBe(false); + expect(util.isCheckedProperty(-1, 1000)).toBe(false); + expect(util.isCheckedProperty(-1, '1000')).toBe(false); + expect(util.isCheckedProperty(-1, -1)).toBe(true); + expect(util.isCheckedProperty(-1, true)).toBe(false); + expect(util.isCheckedProperty(-1, false)).toBe(false); + expect(util.isCheckedProperty(-1, 'true')).toBe(false); + expect(util.isCheckedProperty(-1, 'false')).toBe(false); + }); + + it('should test a="-1"', () => { + expect(util.isCheckedProperty('-1', undefined)).toBe(false); + expect(util.isCheckedProperty('-1', null)).toBe(false); + expect(util.isCheckedProperty('-1', '')).toBe(false); + expect(util.isCheckedProperty('-1', 'string')).toBe(false); + expect(util.isCheckedProperty('-1', 0)).toBe(false); + expect(util.isCheckedProperty('-1', '0')).toBe(false); + expect(util.isCheckedProperty('-1', 1000)).toBe(false); + expect(util.isCheckedProperty('-1', '1000')).toBe(false); + expect(util.isCheckedProperty('-1', -1)).toBe(true); + expect(util.isCheckedProperty('-1', true)).toBe(false); + expect(util.isCheckedProperty('-1', false)).toBe(false); + expect(util.isCheckedProperty('-1', 'true')).toBe(false); + expect(util.isCheckedProperty('-1', 'false')).toBe(false); + }); + + it('should test a="string"', () => { + expect(util.isCheckedProperty('string', undefined)).toBe(false); + expect(util.isCheckedProperty('string', null)).toBe(false); + expect(util.isCheckedProperty('string', '')).toBe(false); + expect(util.isCheckedProperty('string', 'string')).toBe(true); + expect(util.isCheckedProperty('string', 'otherstring')).toBe(false); + expect(util.isCheckedProperty('string', 0)).toBe(false); + expect(util.isCheckedProperty('string', '0')).toBe(false); + expect(util.isCheckedProperty('string', 1000)).toBe(false); + expect(util.isCheckedProperty('string', '1000')).toBe(false); + expect(util.isCheckedProperty('string', -1)).toBe(false); + expect(util.isCheckedProperty('string', true)).toBe(false); + expect(util.isCheckedProperty('string', false)).toBe(false); + expect(util.isCheckedProperty('string', 'true')).toBe(false); + expect(util.isCheckedProperty('string', 'false')).toBe(false); + }); + + }); + describe('isTrueProperty', function() { it('should be true from boolean true', () => { diff --git a/ionic/util/util.ts b/ionic/util/util.ts index e66bf5a50c..ea072202b3 100644 --- a/ionic/util/util.ts +++ b/ionic/util/util.ts @@ -119,6 +119,24 @@ export const isTrueProperty = function(val: any): boolean { return !!val; }; +export const isCheckedProperty = function(a: any, b: any): boolean { + if (a === undefined || a === null || a === '') { + return (b === undefined || b === null || b === ''); + + } else if (a === true || a === 'true') { + return (b === true || b === 'true'); + + } else if (a === false || a === 'false') { + return (b === false || b === 'false'); + + } else if (a === 0 || a === '0') { + return (b === 0 || b === '0'); + } + + // not using strict comparison on purpose + return (a == b); +}; + /** * Convert a string in the format thisIsAString to a slug format this-is-a-string */