Skip to content

Commit

Permalink
fix(MenuButton): Unify behavior of showing/hiding (#4157)
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 7, 2017
1 parent 2ee133f commit c611f9f
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 29 deletions.
29 changes: 2 additions & 27 deletions src/js/control-bar/text-track-controls/captions-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,33 +42,6 @@ class CaptionsButton extends TextTrackButton {
return `vjs-captions-button ${super.buildWrapperCSSClass()}`;
}

/**
* 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 @@ -80,6 +53,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
2 changes: 2 additions & 0 deletions src/js/control-bar/text-track-controls/text-track-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ class TextTrackButton extends TrackButton {
label
}));

this.hideThreshold_ += 1;

const tracks = this.player_.textTracks();

for (let i = 0; i < tracks.length; i++) {
Expand Down
16 changes: 14 additions & 2 deletions src/js/menu/menu-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ class MenuButton extends Component {
this.buttonPressed_ = false;
this.menuButton_.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 @@ -91,6 +91,16 @@ class MenuButton extends Component {
createMenu() {
const menu = new Menu(this.player_, { menuButton: this });

/**
* 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 @@ -99,6 +109,8 @@ class MenuButton extends Component {
tabIndex: -1
});

this.hideThreshold_ += 1;

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

0 comments on commit c611f9f

Please sign in to comment.