Skip to content

Commit

Permalink
@llun fixed menus from throwing when focused when empty. closes #3218
Browse files Browse the repository at this point in the history
  • Loading branch information
Maythee Anegboonlap authored and gkatsev committed Mar 29, 2016
1 parent 6845cbe commit 31b2f9d
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 6 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ CHANGELOG
=========

## HEAD (Unreleased)
_(none)_
* @llun fixed menus from throwing when focused when empty ([view](https://github.com/videojs/video.js/pull/3218))

--------------------

Expand Down
3 changes: 1 addition & 2 deletions src/js/menu/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,13 @@ class Menu extends Component {
*/
focus (item = 0) {
let children = this.children().slice();
let haveTitle = children[0].className &&
let haveTitle = children.length && children[0].className &&
/vjs-menu-title/.test(children[0].className);

if (haveTitle) {
children.shift();
}


if (children.length > 0) {
if (item < 0) {
item = 0;
Expand Down
22 changes: 19 additions & 3 deletions test/unit/menu.test.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,27 @@
import MenuButton from '../../src/js/menu/menu-button.js';
import TestHelpers from './test-helpers.js';
import * as Events from '../../src/js/utils/events.js';

q.module('MenuButton');

test('should place title list item into ul', function() {
var player, menuButton;
q.test('should not throw an error when there is no children', function() {
expect(0);
let player = TestHelpers.makePlayer();

let menuButton = new MenuButton(player);
let el = menuButton.el();

try {
Events.trigger(el, 'click');
} catch (error) {
ok(!error, 'click should not throw anything');
}

player.dispose();
});

q.test('should place title list item into ul', function() {
var player, menuButton;
player = TestHelpers.makePlayer();

menuButton = new MenuButton(player, {
Expand All @@ -18,4 +34,4 @@ test('should place title list item into ul', function() {
ok(titleElement.innerHTML === 'TestTitle', 'title element placed in ul');

player.dispose();
});
});

0 comments on commit 31b2f9d

Please sign in to comment.