From 4367c69752b85c2975f47d7be1e80f1c7e50c74f Mon Sep 17 00:00:00 2001
From: Justin Anastos <justin.anastos@gmail.com>
Date: Thu, 2 Mar 2017 15:24:17 -0500
Subject: [PATCH] fix(MenuButton): Unify behavior of showing/hiding (#3993)

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.
---
 .../text-track-controls/captions-button.js    | 29 ++-----------------
 .../text-track-controls/text-track-button.js  |  1 +
 src/js/menu/menu-button.js                    | 16 ++++++++--
 3 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/src/js/control-bar/text-track-controls/captions-button.js b/src/js/control-bar/text-track-controls/captions-button.js
index 03171ede92..622875352c 100644
--- a/src/js/control-bar/text-track-controls/captions-button.js
+++ b/src/js/control-bar/text-track-controls/captions-button.js
@@ -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
    *
@@ -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);
diff --git a/src/js/control-bar/text-track-controls/text-track-button.js b/src/js/control-bar/text-track-controls/text-track-button.js
index bc2d36c772..02e92557fa 100644
--- a/src/js/control-bar/text-track-controls/text-track-button.js
+++ b/src/js/control-bar/text-track-controls/text-track-button.js
@@ -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();
 
diff --git a/src/js/menu/menu-button.js b/src/js/menu/menu-button.js
index 0936520915..a8e3a6ad1f 100644
--- a/src/js/menu/menu-button.js
+++ b/src/js/menu/menu-button.js
@@ -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();
     }
   }
@@ -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', {
@@ -82,6 +92,8 @@ class MenuButton extends ClickableComponent {
         tabIndex: -1
       });
 
+      this.hideThreshold_ += 1;
+
       menu.children_.unshift(title);
       Dom.insertElFirst(title, menu.contentEl());
     }