Skip to content

Commit

Permalink
fix(MenuButton): Unify behavior of showing/hiding (#3993)
Browse files Browse the repository at this point in the history
Implement a `hideThreshold` property that defaults to 1 so
descendants can override it if necessary. Right now the only
descendant that will override will be `CaptionsButton` because
video.js adds a "captions settings" for emulated text tracks.
  • Loading branch information
justinanastos authored and gkatsev committed Mar 2, 2017
1 parent 072c277 commit 4367c69
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 29 deletions.
29 changes: 2 additions & 27 deletions src/js/control-bar/text-track-controls/captions-button.js
Expand Up @@ -39,33 +39,6 @@ class CaptionsButton extends TextTrackButton {
return `vjs-captions-button ${super.buildCSSClass()}`;
}

/**
* Update caption menu items
*
* @param {EventTarget~Event} [event]
* The `addtrack` or `removetrack` event that caused this function to be
* called.
*
* @listens TextTrackList#addtrack
* @listens TextTrackList#removetrack
*/
update(event) {
let threshold = 2;

super.update();

// if native, then threshold is 1 because no settings button
if (this.player().tech_ && this.player().tech_.featuresNativeTextTracks) {
threshold = 1;
}

if (this.items && this.items.length > threshold) {
this.show();
} else {
this.hide();
}
}

/**
* Create caption menu items
*
Expand All @@ -77,6 +50,8 @@ class CaptionsButton extends TextTrackButton {

if (!(this.player().tech_ && this.player().tech_.featuresNativeTextTracks)) {
items.push(new CaptionSettingsMenuItem(this.player_, {kind: this.kind_}));

this.hideThreshold_ += 1;
}

return super.createItems(items);
Expand Down
Expand Up @@ -40,6 +40,7 @@ class TextTrackButton extends TrackButton {
createItems(items = []) {
// Add an OFF menu item to turn all tracks off
items.push(new OffTextTrackMenuItem(this.player_, {kind: this.kind_}));
this.hideThreshold_ += 1;

const tracks = this.player_.textTracks();

Expand Down
16 changes: 14 additions & 2 deletions src/js/menu/menu-button.js
Expand Up @@ -58,9 +58,9 @@ class MenuButton extends ClickableComponent {
this.buttonPressed_ = false;
this.el_.setAttribute('aria-expanded', 'false');

if (this.items && this.items.length === 0) {
if (this.items && this.items.length <= this.hideThreshold_) {
this.hide();
} else if (this.items && this.items.length > 1) {
} else {
this.show();
}
}
Expand All @@ -74,6 +74,16 @@ class MenuButton extends ClickableComponent {
createMenu() {
const menu = new Menu(this.player_);

/**
* Hide the menu if the number of items is less than or equal to this threshold. This defaults
* to 0 and whenever we add items which can be hidden to the menu we'll increment it. We list
* it here because every time we run `createMenu` we need to reset the value.
*
* @protected
* @type {Number}
*/
this.hideThreshold_ = 0;

// Add a title list item to the top
if (this.options_.title) {
const title = Dom.createEl('li', {
Expand All @@ -82,6 +92,8 @@ class MenuButton extends ClickableComponent {
tabIndex: -1
});

this.hideThreshold_ += 1;

menu.children_.unshift(title);
Dom.insertElFirst(title, menu.contentEl());
}
Expand Down

0 comments on commit 4367c69

Please sign in to comment.