fix(checkbox): remove register/deregisterEventlisteners from foundation #3402
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3402 +/- ##
==========================================
- Coverage 98.41% 98.41% -0.01%
==========================================
Files 123 123
Lines 5183 5180 -3
Branches 638 639 +1
==========================================
- Hits 5101 5098 -3
Misses 82 82
Continue to review full report at Codecov.
|
|
All 353 screenshot tests passed for commit b148d92 vs. |
packages/mdc-checkbox/index.js
Outdated
| this.handleChange_ = () => this.foundation_.handleChange(); | ||
| this.handleAnimationEnd_= () => this.foundation_.handleAnimationEnd(); | ||
| this.nativeCb_.addEventListener('change', this.handleChange_); | ||
| this.root_.addEventListener(getCorrectEventName(window, 'animationend'), this.handleAnimationEnd_); |
There was a problem hiding this comment.
Change this to this.listen (since you're using unlisten in destroy anyway)
packages/mdc-checkbox/foundation.js
Outdated
| @@ -297,6 +291,8 @@ class MDCCheckboxFoundation extends MDCFoundation { | |||
| this.adapter_.setNativeControlAttr( | |||
| strings.ARIA_CHECKED_ATTR, strings.ARIA_CHECKED_INDETERMINATE_VALUE); | |||
| } else { | |||
| // the on/off state does not need to keep track of aria-checked, since | |||
| td.verify(component.foundation_.handleAnimationEnd(), {times: 1}); | ||
| }); | ||
|
|
||
| test('checkbox change event is destroyed on #destroy', () => { |
There was a problem hiding this comment.
event handler is destroyed
| td.verify(component.foundation_.handleChange(), {times: 0}); | ||
| }); | ||
|
|
||
| test('root animationend event is destroyed on #destroy', () => { |
There was a problem hiding this comment.
event handler is destroyed
| (handler) => this.root_.addEventListener(getCorrectEventName(window, 'animationend'), handler), | ||
| deregisterAnimationEndHandler: | ||
| (handler) => this.root_.removeEventListener(getCorrectEventName(window, 'animationend'), handler), | ||
| registerChangeHandler: (handler) => this.nativeCb_.addEventListener('change', handler), |
There was a problem hiding this comment.
You forgot to remove deregisterChangeHandler (I noticed because coverage found it uncovered, but it shouldn't be here anymore)
|
All 353 screenshot tests passed for commit b440ea1 vs. |
|
All 353 screenshot tests passed for commit 0acae16 vs. |
refs #2813