Skip to content

Commit

Permalink
Fix: Properly remove transitionend handler (#499)
Browse files Browse the repository at this point in the history
  • Loading branch information
tonyjin authored Nov 21, 2017
1 parent a8f66ed commit 79b4576
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
18 changes: 14 additions & 4 deletions src/lib/viewers/media/Settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@ class Settings extends EventEmitter {
this.containerEl = containerEl;
this.cache = cache;

// Bind context for callbacks
this.handleTransitionEnd = this.handleTransitionEnd.bind(this);

insertTemplate(this.containerEl, SETTINGS_TEMPLATE, containerEl.querySelector('.bp-media-controls-wrapper'));
this.settingsEl = this.containerEl.querySelector('.bp-media-settings');
this.firstMenuItem = this.settingsEl.querySelectorAll('.bp-media-settings-item')[0];
Expand All @@ -184,9 +187,7 @@ class Settings extends EventEmitter {
this.containerEl.classList.add(CLASS_SETTINGS_AUDIOTRACKS_UNAVAILABLE);

// Allow scrollbars after animations end
this.settingsEl.addEventListener('transitionend', () => {
this.settingsEl.classList.remove('bp-media-settings-in-transition');
});
this.settingsEl.addEventListener('transitionend', this.handleTransitionEnd);

if (Browser.isMobile()) {
this.containerEl.classList.add(CLASS_SETTINGS_AUTOPLAY_UNAVAILABLE);
Expand Down Expand Up @@ -219,7 +220,7 @@ class Settings extends EventEmitter {
destroy() {
if (this.settingsEl) {
removeActivationListener(this.settingsEl, this.menuEventHandler);
this.settingsEl.removeEventListener('transitionend');
this.settingsEl.removeEventListener('transitionend', this.handleTransitionEnd);
}
document.removeEventListener('click', this.blurHandler);
}
Expand Down Expand Up @@ -437,6 +438,15 @@ class Settings extends EventEmitter {
}
}

/**
* Handles transitionend event by removing transition class.
*
* @return {void}
*/
handleTransitionEnd() {
this.settingsEl.classList.remove('bp-media-settings-in-transition');
}

/**
* Returns the selected option in the submenu specified by `type`
*
Expand Down
8 changes: 8 additions & 0 deletions src/lib/viewers/media/__tests__/Settings-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,14 @@ describe('lib/viewers/media/Settings', () => {
});
});

describe('handleTransitionEnd', () => {
it('should remove in transition class', () => {
settings.settingsEl.classList.add('bp-media-settings-in-transition');
settings.handleTransitionEnd();
expect(settings.settingsEl).to.not.have.class('bp-media-settings-in-transition');
});
});

describe('showSubMenu()', () => {
it('should show the speed submenu if speed is selected', () => {
settings.showSubMenu('speed');
Expand Down

0 comments on commit 79b4576

Please sign in to comment.