Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(checkbox): change checkbox event type from change to click and add some logic for IE browser #5316

Merged
merged 12 commits into from
Dec 19, 2019
Merged
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;
candysonya marked this conversation as resolved.
Show resolved Hide resolved
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;
candysonya marked this conversation as resolved.
Show resolved Hide resolved
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