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): remove native control from getters/setters of foundation #3408

Merged
merged 10 commits into from
Aug 28, 2018
12 changes: 12 additions & 0 deletions packages/mdc-checkbox/adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,18 @@ class MDCCheckboxAdapter {

/** @return {boolean} */
isAttachedToDOM() {}

/** @return {boolean} */
isIndeterminate() {}

/** @return {boolean} */
isChecked() {}

/** @return {boolean} */
hasNativeControl() {}

/** @param {boolean} disabled */
setNativeControlDisabled(disabled) {}
}

export default MDCCheckboxAdapter;
60 changes: 14 additions & 46 deletions packages/mdc-checkbox/foundation.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ class MDCCheckboxFoundation extends MDCFoundation {
getNativeControl: () => /* !MDCSelectionControlState */ {},
forceLayout: () => {},
isAttachedToDOM: () => /* boolean */ {},
isIndeterminate: () => /* boolean */ {},
isChecked: () => /* boolean */ {},
hasNativeControl: () => /* boolean */ {},
setNativeControlDisabled: (/* disabled: boolean */) => {},
});
}

Expand All @@ -88,7 +92,7 @@ class MDCCheckboxFoundation extends MDCFoundation {

/** @override */
init() {
this.currentCheckState_ = this.determineCheckState_(this.getNativeControl_());
this.currentCheckState_ = this.determineCheckState_();
this.updateAriaChecked_();
this.adapter_.addClass(cssClasses.UPGRADED);
this.adapter_.registerChangeHandler(this.changeHandler_);
Expand All @@ -101,51 +105,16 @@ class MDCCheckboxFoundation extends MDCFoundation {
this.uninstallPropertyChangeHooks_();
}

/** @return {boolean} */
isChecked() {
return this.getNativeControl_().checked;
}

/** @param {boolean} checked */
setChecked(checked) {
this.getNativeControl_().checked = checked;
}

/** @return {boolean} */
isIndeterminate() {
return this.getNativeControl_().indeterminate;
}

/** @param {boolean} indeterminate */
setIndeterminate(indeterminate) {
this.getNativeControl_().indeterminate = indeterminate;
}

/** @return {boolean} */
isDisabled() {
return this.getNativeControl_().disabled;
}

/** @param {boolean} disabled */
setDisabled(disabled) {
this.getNativeControl_().disabled = disabled;
this.adapter_.setNativeControlDisabled(disabled);
if (disabled) {
this.adapter_.addClass(cssClasses.DISABLED);
} else {
this.adapter_.removeClass(cssClasses.DISABLED);
}
}

/** @return {?string} */
getValue() {
return this.getNativeControl_().value;
}

/** @param {?string} value */
setValue(value) {
this.getNativeControl_().value = value;
}

/**
* Handles the animationend event for the checkbox
*/
Expand Down Expand Up @@ -204,12 +173,12 @@ class MDCCheckboxFoundation extends MDCFoundation {

/** @private */
transitionCheckState_() {
const nativeCb = this.adapter_.getNativeControl();
if (!nativeCb) {
if (!this.adapter_.hasNativeControl()) {
return;
}
const oldState = this.currentCheckState_;
const newState = this.determineCheckState_(nativeCb);
const newState = this.determineCheckState_();

if (oldState === newState) {
return;
}
Expand All @@ -236,21 +205,20 @@ class MDCCheckboxFoundation extends MDCFoundation {
}

/**
* @param {!MDCSelectionControlState} nativeCb
* @return {string}
* @private
*/
determineCheckState_(nativeCb) {
determineCheckState_() {
const {
TRANSITION_STATE_INDETERMINATE,
TRANSITION_STATE_CHECKED,
TRANSITION_STATE_UNCHECKED,
} = strings;

if (nativeCb.indeterminate) {
if (this.adapter_.isIndeterminate()) {
return TRANSITION_STATE_INDETERMINATE;
}
return nativeCb.checked ? TRANSITION_STATE_CHECKED : TRANSITION_STATE_UNCHECKED;
return this.adapter_.isChecked() ? TRANSITION_STATE_CHECKED : TRANSITION_STATE_UNCHECKED;
}

/**
Expand Down Expand Up @@ -292,8 +260,8 @@ class MDCCheckboxFoundation extends MDCFoundation {
}

updateAriaChecked_() {
// Ensure aria-checked is set to mixed if checkbox is in indeterminate state.
if (this.isIndeterminate()) {
// Ensure aria-checked is set to if checkbox is in indeterminate state.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You accidentally deleted mixed somehow

if (this.adapter_.isIndeterminate()) {
this.adapter_.setNativeControlAttr(
strings.ARIA_CHECKED_ATTR, strings.ARIA_CHECKED_INDETERMINATE_VALUE);
} else {
Expand Down
34 changes: 27 additions & 7 deletions packages/mdc-checkbox/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,39 +88,59 @@ class MDCCheckbox extends MDCComponent {
registerChangeHandler: (handler) => this.nativeCb_.addEventListener('change', handler),
deregisterChangeHandler: (handler) => this.nativeCb_.removeEventListener('change', handler),
getNativeControl: () => this.nativeCb_,
isIndeterminate: () => this.isIndeterminate_(),
isChecked: () => this.isChecked_(),
hasNativeControl: () => !!this.nativeCb_,
setNativeControlDisabled: (disabled) => this.nativeCb_.disabled = disabled,
forceLayout: () => this.root_.offsetWidth,
isAttachedToDOM: () => Boolean(this.root_.parentNode),
});
}

/**
* @return {boolean}
* @private
*/
isIndeterminate_() {
return this.nativeCb_.indeterminate;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move these to the get methods and then update the adapter to use the public get ? Seems like we don't really need the private function.

}

/**
* @return {boolean}
* @private
*/
isChecked_() {
return this.nativeCb_.checked;
}

/** @return {!MDCRipple} */
get ripple() {
return this.ripple_;
}

/** @return {boolean} */
get checked() {
return this.foundation_.isChecked();
return this.isChecked_();
}

/** @param {boolean} checked */
set checked(checked) {
this.foundation_.setChecked(checked);
this.nativeCb_.checked = checked;
}

/** @return {boolean} */
get indeterminate() {
return this.foundation_.isIndeterminate();
return this.isIndeterminate_();
}

/** @param {boolean} indeterminate */
set indeterminate(indeterminate) {
this.foundation_.setIndeterminate(indeterminate);
this.nativeCb_.indeterminate = indeterminate;
}

/** @return {boolean} */
get disabled() {
return this.foundation_.isDisabled();
return this.nativeCb_.disabled;
}

/** @param {boolean} disabled */
Expand All @@ -130,12 +150,12 @@ class MDCCheckbox extends MDCComponent {

/** @return {?string} */
get value() {
return this.foundation_.getValue();
return this.nativeCb_.value;
}

/** @param {?string} value */
set value(value) {
this.foundation_.setValue(value);
this.nativeCb_.value = value;
}

destroy() {
Expand Down
Loading