feat: Refactoring chapters button handling and fixing several issues (#…

…3472)

* Refactored ChaptersButton, broke logic into several methods.
* Fixed the issues in #3447 about in some browsers tracks have an empty cues array instead of null. * Now we always subscribe to load event, and force an update. Also, track changes are handled, so chapters track can now be changed at runtime.
* Fixed the issue in #3447 about chapters menu items are not selectable. Now automatic update of the selected item based on player time works fine.
* Implemented the usage of the chapters track's label attribute as menu title, if it's present. If not, we fall back to the localized "Chapters" title.
* Refined the menu styling, so that vjs-menu-title telement won't get the hover effect, It would confuse users, because they might believe that the title item is a clickable item too.

src/css/components/menu/_menu.scss

@@ -35,8 +35,8 @@
   text-transform: lowercase;
 }
 
-.vjs-menu li:focus,
-.vjs-menu li:hover {
+.vjs-menu li.vjs-menu-item:focus,
+.vjs-menu li.vjs-menu-item:hover {
   outline: 0;
   @include background-color-with-alpha($secondary-background-color, $secondary-background-transparency);
 }

src/js/control-bar/text-track-controls/chapters-button.js

@@ -3,10 +3,7 @@
  */
 import TextTrackButton from './text-track-button.js';
 import Component from '../../component.js';
-import TextTrackMenuItem from './text-track-menu-item.js';
 import ChaptersTrackMenuItem from './chapters-track-menu-item.js';
-import Menu from '../../menu/menu.js';
-import * as Dom from '../../utils/dom.js';
 import toTitleCase from '../../utils/to-title-case.js';
 
 /**
@@ -37,108 +34,105 @@ class ChaptersButton extends TextTrackButton {
     return `vjs-chapters-button ${super.buildCSSClass()}`;
   }
 
-  /**
-   * Create a menu item for each text track
-   *
-   * @return {Array} Array of menu items
-   * @method createItems
-   */
-  createItems() {
-    const items = [];
-    const tracks = this.player_.textTracks();
+  update(event) {
+    if (!this.track_ || (event && (event.type === 'addtrack' || event.type === 'removetrack'))) {
+      this.setTrack(this.findChaptersTrack());
+    }
+    super.update();
+  }
 
-    if (!tracks) {
-      return items;
+  setTrack(track) {
+    if (this.track_ === track) {
+      return;
     }
 
-    for (let i = 0; i < tracks.length; i++) {
-      const track = tracks[i];
+    if (!this.updateHandler_) {
+      this.updateHandler_ = this.update.bind(this);
+    }
 
-      if (track.kind === this.kind_) {
-        items.push(new TextTrackMenuItem(this.player_, {track}));
+    // here this.track_ refers to the old track instance
+    if (this.track_) {
+      const remoteTextTrackEl = this.player_.remoteTextTrackEls().getTrackElementByTrack_(this.track_);
+
+      if (remoteTextTrackEl) {
+        remoteTextTrackEl.removeEventListener('load', this.updateHandler_);
       }
+
+      this.track_ = null;
     }
 
-    return items;
+    this.track_ = track;
+
+    // here this.track_ refers to the new track instance
+    if (this.track_) {
+      this.track_.mode = 'hidden';
+
+      const remoteTextTrackEl = this.player_.remoteTextTrackEls().getTrackElementByTrack_(this.track_);
+
+      if (remoteTextTrackEl) {
+        remoteTextTrackEl.addEventListener('load', this.updateHandler_);
+      }
+    }
   }
 
-  /**
-   * Create menu from chapter buttons
-   *
-   * @return {Menu} Menu of chapter buttons
-   * @method createMenu
-   */
-  createMenu() {
+  findChaptersTrack() {
     const tracks = this.player_.textTracks() || [];
-    let chaptersTrack;
-    let items = this.items || [];
 
     for (let i = tracks.length - 1; i >= 0; i--) {
-
       // We will always choose the last track as our chaptersTrack
       const track = tracks[i];
 
       if (track.kind === this.kind_) {
-        chaptersTrack = track;
-
-        break;
+        return track;
       }
     }
+  }
 
-    let menu = this.menu;
-
-    if (menu === undefined) {
-      menu = new Menu(this.player_);
-
-      const title = Dom.createEl('li', {
-        className: 'vjs-menu-title',
-        innerHTML: toTitleCase(this.kind_),
-        tabIndex: -1
-      });
-
-      menu.children_.unshift(title);
-      Dom.insertElFirst(title, menu.contentEl());
-    } else {
-      // We will empty out the menu children each time because we want a
-      // fresh new menu child list each time
-      items.forEach(item => menu.removeChild(item));
-      // Empty out the ChaptersButton menu items because we no longer need them
-      items = [];
+  getMenuCaption() {
+    if (this.track_ && this.track_.label) {
+      return this.track_.label;
     }
+    return this.localize(toTitleCase(this.kind_));
+  }
 
-    if (chaptersTrack && (chaptersTrack.cues === null || chaptersTrack.cues === undefined)) {
-      chaptersTrack.mode = 'hidden';
+  /**
+   * Create menu from chapter track
+   *
+   * @return {Menu} Menu of chapter buttons
+   * @method createMenu
+   */
+  createMenu() {
+    this.options_.title = this.getMenuCaption();
+    return super.createMenu();
+  }
 
-      const remoteTextTrackEl = this.player_.remoteTextTrackEls().getTrackElementByTrack_(chaptersTrack);
+  /**
+   * Create a menu item for each chapter cue
+   *
+   * @return {Array} Array of menu items
+   * @method createItems
+   */
+  createItems() {
+    const items = [];
 
-      if (remoteTextTrackEl) {
-        remoteTextTrackEl.addEventListener('load', (event) => this.update());
-      }
+    if (!this.track_) {
+      return items;
     }
 
-    if (chaptersTrack && chaptersTrack.cues && chaptersTrack.cues.length > 0) {
-      const cues = chaptersTrack.cues;
-
-      for (let i = 0, l = cues.length; i < l; i++) {
-        const cue = cues[i];
+    const cues = this.track_.cues;
 
-        const mi = new ChaptersTrackMenuItem(this.player_, {
-          cue,
-          track: chaptersTrack
-        });
+    if (!cues) {
+      return items;
+    }
 
-        items.push(mi);
+    for (let i = 0, l = cues.length; i < l; i++) {
+      const cue = cues[i];
+      const mi = new ChaptersTrackMenuItem(this.player_, { track: this.track_, cue });
 
-        menu.addChild(mi);
-      }
+      items.push(mi);
     }
 
-    if (items.length > 0) {
-      this.show();
-    }
-    // Assigning the value of items back to this.items for next iteration
-    this.items = items;
-    return menu;
+    return items;
   }
 }
 

src/js/control-bar/text-track-controls/chapters-track-menu-item.js

@@ -21,6 +21,7 @@ class ChaptersTrackMenuItem extends MenuItem {
     const currentTime = player.currentTime();
 
     // Modify options for parent MenuItem class's init.
+    options.selectable = true;
     options.label = cue.text;
     options.selected = (cue.startTime <= currentTime && currentTime < cue.endTime);
     super(player, options);

test/unit/test-helpers.js

@@ -130,6 +130,32 @@ const TestHelpers = {
       props.length;
 
     return run;
+  },
+
+  /**
+   * Triggers an event on a DOM node natively.
+   *
+   * @param  {Element} element
+   * @param  {string} eventType
+   */
+  triggerDomEvent(element, eventType) {
+    let event;
+
+    if (document.createEvent) {
+      event = document.createEvent('HTMLEvents');
+      event.initEvent(eventType, true, true);
+    } else {
+      event = document.createEventObject();
+      event.eventType = eventType;
+    }
+
+    event.eventName = eventType;
+
+    if (document.createEvent) {
+      element.dispatchEvent(event);
+    } else {
+      element.fireEvent('on' + event.eventType, event);
+    }
   }
 };
 

test/unit/tracks/text-track-controls.test.js

@@ -294,3 +294,154 @@ if (!browser.IS_IE8) {
     player.dispose();
   });
 }
+
+const chaptersTrack = {
+  kind: 'chapters',
+  label: 'Test Chapters'
+};
+
+test('chapters should not be displayed when text tracks list is empty', function() {
+  const player = TestHelpers.makePlayer();
+
+  ok(player.controlBar.chaptersButton.hasClass('vjs-hidden'), 'control is not displayed');
+  equal(player.textTracks().length, 0, 'textTracks is empty');
+
+  player.dispose();
+});
+
+test('chapters should not be displayed when there is chapters track but no cues', function() {
+  const player = TestHelpers.makePlayer({
+    tracks: [chaptersTrack]
+  });
+
+  this.clock.tick(1000);
+
+  ok(player.controlBar.chaptersButton.hasClass('vjs-hidden'), 'chapters menu is not displayed');
+  equal(player.textTracks().length, 1, 'textTracks contains one item');
+
+  player.dispose();
+});
+
+test('chapters should be displayed when cues added to initial track and button updated', function() {
+  const player = TestHelpers.makePlayer({
+    tracks: [chaptersTrack]
+  });
+
+  this.clock.tick(1000);
+
+  const chapters = player.textTracks()[0];
+
+  chapters.addCue({
+    startTime: 0,
+    endTime: 2,
+    text: 'Chapter 1'
+  });
+  chapters.addCue({
+    startTime: 2,
+    endTime: 4,
+    text: 'Chapter 2'
+  });
+  equal(chapters.cues.length, 2);
+
+  player.controlBar.chaptersButton.update();
+
+  ok(!player.controlBar.chaptersButton.hasClass('vjs-hidden'), 'chapters menu is displayed');
+
+  const menuItems = player.controlBar.chaptersButton.items;
+
+  equal(menuItems.length, 2, 'menu contains two item');
+
+  player.dispose();
+});
+
+test('chapters should be displayed when a track and its cures added and button updated', function() {
+  const player = TestHelpers.makePlayer();
+
+  this.clock.tick(1000);
+
+  const chapters = player.addTextTrack('chapters', 'Test Chapters', 'en');
+
+  chapters.addCue({
+    startTime: 0,
+    endTime: 2,
+    text: 'Chapter 1'
+  });
+  chapters.addCue({
+    startTime: 2,
+    endTime: 4,
+    text: 'Chapter 2'
+  });
+  equal(chapters.cues.length, 2);
+
+  player.controlBar.chaptersButton.update();
+
+  ok(!player.controlBar.chaptersButton.hasClass('vjs-hidden'), 'chapters menu is displayed');
+
+  const menuItems = player.controlBar.chaptersButton.items;
+
+  equal(menuItems.length, 2, 'menu contains two item');
+
+  player.dispose();
+});
+
+test('chapters menu should use track label as menu title', function() {
+  const player = TestHelpers.makePlayer({
+    tracks: [chaptersTrack]
+  });
+
+  this.clock.tick(1000);
+
+  const chapters = player.textTracks()[0];
+
+  chapters.addCue({
+    startTime: 0,
+    endTime: 2,
+    text: 'Chapter 1'
+  });
+  chapters.addCue({
+    startTime: 2,
+    endTime: 4,
+    text: 'Chapter 2'
+  });
+  equal(chapters.cues.length, 2);
+
+  player.controlBar.chaptersButton.update();
+
+  const menu = player.controlBar.chaptersButton.menu;
+  const menuTitle = menu.contentEl().firstChild.textContent;
+
+  equal(menuTitle, 'Test Chapters', 'menu gets track label as title');
+
+  player.dispose();
+});
+
+test('chapters should be displayed when remote track added and load event fired', function() {
+  const player = TestHelpers.makePlayer();
+
+  this.clock.tick(1000);
+
+  const chaptersEl = player.addRemoteTextTrack(chaptersTrack);
+
+  chaptersEl.track.addCue({
+    startTime: 0,
+    endTime: 2,
+    text: 'Chapter 1'
+  });
+  chaptersEl.track.addCue({
+    startTime: 2,
+    endTime: 4,
+    text: 'Chapter 2'
+  });
+
+  equal(chaptersEl.track.cues.length, 2);
+
+  TestHelpers.triggerDomEvent(chaptersEl, 'load');
+
+  ok(!player.controlBar.chaptersButton.hasClass('vjs-hidden'), 'chapters menu is displayed');
+
+  const menuItems = player.controlBar.chaptersButton.items;
+
+  equal(menuItems.length, 2, 'menu contains two item');
+
+  player.dispose();
+});