Skip to content

Commit

Permalink
fix(checkbox): change checkbox event type from change to click and ad…
Browse files Browse the repository at this point in the history
…d some logic for IE browser (#5316)

Closes #4893

BREAKING CHANGE: remove event listener for 'change' and add event listener for 'click'.

- Add handleClick() method in foundation to handle click event.
- Add setCheck() method into component to change check status.
  • Loading branch information
candysonya authored Dec 19, 2019
1 parent bac43eb commit 2e491de
Show file tree
Hide file tree
Showing 6 changed files with 308 additions and 16 deletions.
4 changes: 3 additions & 1 deletion packages/mdc-checkbox/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ Method Signature | Description
`isIndeterminate() => boolean` | Returns true if the component is in the indeterminate state.
`isChecked() => boolean` | Returns true if the component is checked.
`hasNativeControl() => boolean` | Returns true if the input is present in the component.
`setChecked(checkStatus: boolean) => void` | Sets the check status of the component.
`setNativeControlDisabled(disabled: boolean) => void` | Sets the input to disabled.
`setNativeControlAttr(attr: string, value: string) => void` | Sets an HTML attribute to the given value on the native input element.
`removeNativeControlAttr(attr: string) => void` | Removes an attribute from the native input element.
Expand All @@ -192,4 +193,5 @@ Method Signature | Description
--- | ---
`setDisabled(disabled: boolean) => void` | Updates the `disabled` property on the underlying input. Does nothing when the underlying input is not present.
`handleAnimationEnd() => void` | `animationend` event handler that should be applied to the root element.
`handleChange() => void` | `change` event handler that should be applied to the checkbox element.
`handleClick() => void` | `click` event handler that should be applied to the checkbox element.
`handleChange() => void` | Handles check status changes.
1 change: 1 addition & 0 deletions packages/mdc-checkbox/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export interface MDCCheckboxAdapter {
isIndeterminate(): boolean;
removeClass(className: string): void;
removeNativeControlAttr(attr: string): void;
setChecked(checkStatus: boolean): void;
setNativeControlAttr(attr: string, value: string): void;
setNativeControlDisabled(disabled: boolean): void;
}
9 changes: 5 additions & 4 deletions packages/mdc-checkbox/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,20 +86,20 @@ export class MDCCheckbox extends MDCComponent<MDCCheckboxFoundation> implements
root_!: Element; // assigned in MDCComponent constructor

private readonly ripple_: MDCRipple = this.createRipple_();
private handleChange_!: EventListener; // assigned in initialSyncWithDOM()
private handleClick_!: EventListener; // assigned in initialSyncWithDOM()
private handleAnimationEnd_!: EventListener; // assigned in initialSyncWithDOM()

initialSyncWithDOM() {
this.handleChange_ = () => this.foundation_.handleChange();
this.handleClick_ = () => this.foundation_.handleClick();
this.handleAnimationEnd_ = () => this.foundation_.handleAnimationEnd();
this.nativeControl_.addEventListener('change', this.handleChange_);
this.nativeControl_.addEventListener('click', this.handleClick_);
this.listen(getCorrectEventName(window, 'animationend'), this.handleAnimationEnd_);
this.installPropertyChangeHooks_();
}

destroy() {
this.ripple_.destroy();
this.nativeControl_.removeEventListener('change', this.handleChange_);
this.nativeControl_.removeEventListener('click', this.handleClick_);
this.unlisten(getCorrectEventName(window, 'animationend'), this.handleAnimationEnd_);
this.uninstallPropertyChangeHooks_();
super.destroy();
Expand All @@ -117,6 +117,7 @@ export class MDCCheckbox extends MDCComponent<MDCCheckboxFoundation> implements
isIndeterminate: () => this.indeterminate,
removeClass: (className) => this.root_.classList.remove(className),
removeNativeControlAttr: (attr) => this.nativeControl_.removeAttribute(attr),
setChecked: (checkStatus) => this.checked = checkStatus,
setNativeControlAttr: (attr, value) => this.nativeControl_.setAttribute(attr, value),
setNativeControlDisabled: (disabled) => this.nativeControl_.disabled = disabled,
};
Expand Down
26 changes: 25 additions & 1 deletion packages/mdc-checkbox/foundation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,17 @@ export class MDCCheckboxFoundation extends MDCFoundation<MDCCheckboxAdapter> {
isIndeterminate: () => false,
removeClass: () => undefined,
removeNativeControlAttr: () => undefined,
setChecked: () => undefined,
setNativeControlAttr: () => undefined,
setNativeControlDisabled: () => undefined,
};
}

private currentCheckState_ = strings.TRANSITION_STATE_INIT;
// Native checkboxes can only have two real states: checked/true or unchecked/false
// The indeterminate state is visual only.
// See https://stackoverflow.com/a/33529024 for more details.
private realCheckState_ = false;
private currentAnimationClass_ = '';
private animEndLatchTimer_ = 0;
private enableAnimationEndHandler_ = false;
Expand All @@ -64,6 +69,7 @@ export class MDCCheckboxFoundation extends MDCFoundation<MDCCheckboxAdapter> {

init() {
this.currentCheckState_ = this.determineCheckState_();
this.realCheckState_ = this.adapter_.isChecked();
this.updateAriaChecked_();
this.adapter_.addClass(cssClasses.UPGRADED);
}
Expand Down Expand Up @@ -98,7 +104,20 @@ export class MDCCheckboxFoundation extends MDCFoundation<MDCCheckboxAdapter> {
}

/**
* Handles the change event for the checkbox
* Handles the click event for the checkbox
*/
handleClick() {
// added for IE browser to fix compatibility issue:
// https://github.com/material-components/material-components-web/issues/4893
const {TRANSITION_STATE_INDETERMINATE} = strings;
if (this.currentCheckState_ === TRANSITION_STATE_INDETERMINATE) {
this.adapter_.setChecked(!this.realCheckState_);
}
this.transitionCheckState_();
}

/**
* Handles the actions after check status changes
*/
handleChange() {
this.transitionCheckState_();
Expand All @@ -118,11 +137,16 @@ export class MDCCheckboxFoundation extends MDCFoundation<MDCCheckboxAdapter> {
this.updateAriaChecked_();

const {TRANSITION_STATE_UNCHECKED} = strings;
const {TRANSITION_STATE_CHECKED} = strings;
const {SELECTED} = cssClasses;
if (newState === TRANSITION_STATE_UNCHECKED) {
this.adapter_.removeClass(SELECTED);
this.realCheckState_ = false;
} else {
this.adapter_.addClass(SELECTED);
if (newState === TRANSITION_STATE_CHECKED) {
this.realCheckState_ = true;
}
}

// Check to ensure that there isn't a previously existing animation class, in case for example
Expand Down
30 changes: 22 additions & 8 deletions packages/mdc-checkbox/test/component.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,11 @@ describe('MDCCheckbox', () => {
expect(component.ripple instanceof MDCRipple).toBeTruthy();
});

it('checkbox change event calls #foundation.handleChange', () => {
it('checkbox click event calls #foundation.handleClick', () => {
const {cb, component} = setupTest();
(component as any).foundation_.handleChange = jasmine.createSpy();
emitEvent(cb, 'change');
expect((component as any).foundation_.handleChange).toHaveBeenCalled();
(component as any).foundation_.handleClick = jasmine.createSpy();
emitEvent(cb, 'click');
expect((component as any).foundation_.handleClick).toHaveBeenCalled();
});

it('root animationend event calls #foundation.handleAnimationEnd', () => {
Expand All @@ -178,12 +178,12 @@ describe('MDCCheckbox', () => {
expect(mockFoundation.handleChange).toHaveBeenCalled();
});

it('checkbox change event handler is destroyed on #destroy', () => {
it('checkbox click event handler is destroyed on #destroy', () => {
const {cb, component} = setupTest();
(component as any).foundation_.handleChange = jasmine.createSpy();
(component as any).foundation_.handleClick = jasmine.createSpy();
component.destroy();
emitEvent(cb, 'change');
expect((component as any).foundation_.handleChange).not.toHaveBeenCalled();
emitEvent(cb, 'click');
expect((component as any).foundation_.handleClick).not.toHaveBeenCalled();
});

it('root animationend event handler is destroyed on #destroy', () => {
Expand Down Expand Up @@ -304,6 +304,20 @@ describe('MDCCheckbox', () => {
.toBe(false);
});

it('#adapter.setChecked returns true when checkbox is checked', () => {
const {cb, component} = setupTest();
(component.getDefaultFoundation() as any)
.adapter_.setChecked(true);
expect(cb.checked).toBe(true);
});

it('#adapter.setChecked returns false when checkbox is not checked', () => {
const {cb, component} = setupTest();
(component.getDefaultFoundation() as any)
.adapter_.setChecked(false);
expect(cb.checked).toBe(false);
});

it('#adapter.hasNativeControl returns true when checkbox exists', () => {
const {component} = setupTest();
expect(
Expand Down
Loading

0 comments on commit 2e491de

Please sign in to comment.