From 80d8f2623c6c3fafa20dc39e453f5d343ca291f8 Mon Sep 17 00:00:00 2001 From: Erik Yuzwa Date: Tue, 1 Sep 2015 14:37:58 -0600 Subject: [PATCH 01/14] add component to specific index and tests --- src/js/component.js | 9 +++++++-- test/unit/component.test.js | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/js/component.js b/src/js/component.js index f79dd50ec3..e4ac02ccc2 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -339,10 +339,11 @@ class Component { * * @param {String|Component} child The class name or instance of a child to add * @param {Object=} options Options, including options to be passed to children of the child. + * @param {Number} index into our children array to attempt to add the child * @return {Component} The child component (created by this process if a string was used) * @method addChild */ - addChild(child, options={}) { + addChild(child, options={}, index='undefined') { let component; let componentName; @@ -379,7 +380,11 @@ class Component { component = child; } - this.children_.push(component); + if (typeof index === 'number') { + this.children_.splice(index, 0, component); + } else { + this.children_.push(component); + } if (typeof component.id === 'function') { this.childIndex_[component.id()] = component; diff --git a/test/unit/component.test.js b/test/unit/component.test.js index 146a12fd45..6da5a9aa9f 100644 --- a/test/unit/component.test.js +++ b/test/unit/component.test.js @@ -40,6 +40,38 @@ test('should add a child component', function(){ ok(comp.getChildById(child.id()) === child); }); +test('should add a child component to an index', function(){ + var comp = new Component(getFakePlayer()); + + var child = comp.addChild('component'); + + ok(comp.children().length === 1); + ok(comp.children()[0] === child); + + var child0 = comp.addChild('component', {}, 0); + ok(comp.children().length === 2); + ok(comp.children()[0] === child0); + ok(comp.children()[1] === child); + + var child1 = comp.addChild('component', {}, '2'); + ok(comp.children().length === 3); + ok(comp.children()[2] === child1); + + var child2 = comp.addChild('component', {}, null); + ok(comp.children().length === 4); + ok(comp.children()[3] === child2); + + var child3 = comp.addChild('component', {}, undefined); + ok(comp.children().length === 5); + ok(comp.children()[4] === child3); + + var child4 = comp.addChild('component', {}, -1); + ok(comp.children().length === 6); + ok(comp.children()[4] === child4); + ok(comp.children()[5] === child3); + +}); + test('should init child components from options', function(){ var comp = new Component(getFakePlayer(), { children: { From ba0065a9e67f2b7cf60d75f14a8231650b9a539a Mon Sep 17 00:00:00 2001 From: heff Date: Wed, 2 Sep 2015 12:45:26 -0700 Subject: [PATCH 02/14] Minor changes to #2540 --- src/js/component.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/js/component.js b/src/js/component.js index e4ac02ccc2..db76b2a256 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -343,7 +343,7 @@ class Component { * @return {Component} The child component (created by this process if a string was used) * @method addChild */ - addChild(child, options={}, index='undefined') { + addChild(child, options={}, index=this.children_.length) { let component; let componentName; @@ -380,11 +380,7 @@ class Component { component = child; } - if (typeof index === 'number') { - this.children_.splice(index, 0, component); - } else { - this.children_.push(component); - } + this.children_.splice(index, 0, component); if (typeof component.id === 'function') { this.childIndex_[component.id()] = component; @@ -401,7 +397,7 @@ class Component { // Add the UI object's element to the container div (box) // Having an element is not required if (typeof component.el === 'function' && component.el()) { - this.contentEl().appendChild(component.el()); + this.contentEl().insertBefore(component.el(), this.contentEl().children[index]); } // Return so it can stored on parent object if desired. From 0527665fe09cbeaa70a7be6d6732cba5f1ff863f Mon Sep 17 00:00:00 2001 From: Erik Yuzwa Date: Wed, 2 Sep 2015 14:45:43 -0600 Subject: [PATCH 03/14] fixed addChild tests --- test/unit/component.test.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/test/unit/component.test.js b/test/unit/component.test.js index 6da5a9aa9f..c02821de97 100644 --- a/test/unit/component.test.js +++ b/test/unit/component.test.js @@ -57,18 +57,14 @@ test('should add a child component to an index', function(){ ok(comp.children().length === 3); ok(comp.children()[2] === child1); - var child2 = comp.addChild('component', {}, null); + var child2 = comp.addChild('component', {}, undefined); ok(comp.children().length === 4); ok(comp.children()[3] === child2); - var child3 = comp.addChild('component', {}, undefined); + var child3 = comp.addChild('component', {}, -1); ok(comp.children().length === 5); - ok(comp.children()[4] === child3); - - var child4 = comp.addChild('component', {}, -1); - ok(comp.children().length === 6); - ok(comp.children()[4] === child4); - ok(comp.children()[5] === child3); + ok(comp.children()[3] === child3); + ok(comp.children()[4] === child2); }); From d763d0ef115f915034d919bfb8890bfb8a0d15fa Mon Sep 17 00:00:00 2001 From: Erik Yuzwa Date: Wed, 2 Sep 2015 23:24:56 -0600 Subject: [PATCH 04/14] updated failing test with proper index but is it 'right'? --- test/unit/player.test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/unit/player.test.js b/test/unit/player.test.js index 6a91caf089..1ddc562e97 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -271,7 +271,9 @@ test('should load a media controller', function(){ ] }); - ok(player.el().children[0].className.indexOf('vjs-tech') !== -1, 'media controller loaded'); + var it = player.el().children; + //ok(player.el().children[0].className.indexOf('vjs-tech') !== -1, 'media controller loaded'); + ok(player.el().children[it.length-1].className.indexOf('vjs-tech') !== -1, 'media controller loaded'); player.dispose(); }); From 3b0ba5fbb7ff074d43b14fdf2f05414ad1064681 Mon Sep 17 00:00:00 2001 From: Erik Yuzwa Date: Thu, 3 Sep 2015 21:09:26 -0600 Subject: [PATCH 05/14] making use of component.addChild using the updated addChild to add our vjs-tech into our DOM children array --- src/js/player.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/js/player.js b/src/js/player.js index 74cee8ba30..0d9c19ea83 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -298,7 +298,17 @@ class Player extends Component { if (tag.parentNode) { tag.parentNode.insertBefore(el, tag); } - Dom.insertElFirst(tag, el); // Breaks iPhone, fixed in HTML5 setup. + + //Dom.insertElFirst(tag, el); // Breaks iPhone, fixed in HTML5 setup. + + // append the DOM el function to make use of our component.addChild method + // to properly be injected into the children Array + tag.el = function () { + return this; + }; + + // add our tag as the first child + this.addChild(tag, {}, 0); this.el_ = el; From 0f022028119afaddd1e5a7a9c958b82f8e38f217 Mon Sep 17 00:00:00 2001 From: Erik Yuzwa Date: Thu, 3 Sep 2015 21:09:54 -0600 Subject: [PATCH 06/14] restored original media controller test --- test/unit/player.test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/unit/player.test.js b/test/unit/player.test.js index 1ddc562e97..6a91caf089 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -271,9 +271,7 @@ test('should load a media controller', function(){ ] }); - var it = player.el().children; - //ok(player.el().children[0].className.indexOf('vjs-tech') !== -1, 'media controller loaded'); - ok(player.el().children[it.length-1].className.indexOf('vjs-tech') !== -1, 'media controller loaded'); + ok(player.el().children[0].className.indexOf('vjs-tech') !== -1, 'media controller loaded'); player.dispose(); }); From 5f02d1fcefeada2e2370500d93eef3adc205dea1 Mon Sep 17 00:00:00 2001 From: Erik Yuzwa Date: Mon, 9 Nov 2015 20:48:42 -0700 Subject: [PATCH 07/14] rewrote child node insertion into dom --- src/js/component.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/js/component.js b/src/js/component.js index db76b2a256..e42c2cc15c 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -397,7 +397,9 @@ class Component { // Add the UI object's element to the container div (box) // Having an element is not required if (typeof component.el === 'function' && component.el()) { - this.contentEl().insertBefore(component.el(), this.contentEl().children[index]); + var childNodes = this.contentEl().children; + var refNode = childNodes[index] || undefined; + this.contentEl().insertBefore(component.el(), refNode); } // Return so it can stored on parent object if desired. From c5442c88edf14acf5d0db6505a8ae36904cac017 Mon Sep 17 00:00:00 2001 From: Erik Yuzwa Date: Tue, 12 Jan 2016 12:45:53 -0700 Subject: [PATCH 08/14] updated for webstorm ide --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 4f3d583204..b818b145b0 100644 --- a/.gitignore +++ b/.gitignore @@ -28,3 +28,5 @@ test/coverage/* .sass-cache dist/* + +.idea/ From d25a874d3828ba2e58e1e8dc547d544eee572c9f Mon Sep 17 00:00:00 2001 From: Erik Yuzwa Date: Fri, 15 Jan 2016 20:28:14 -0700 Subject: [PATCH 09/14] fixed commented test --- test/unit/component.test.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/unit/component.test.js b/test/unit/component.test.js index 5df60ddb9d..978ac07636 100644 --- a/test/unit/component.test.js +++ b/test/unit/component.test.js @@ -80,17 +80,18 @@ test('should add a child component to an index', function(){ ok(comp.children().length === 5); ok(comp.children()[3] === child3); ok(comp.children()[4] === child2); +}); +test('addChild should throw if the child does not exist', function() { + var comp = new Component(getFakePlayer()); -//test('addChild should throw if the child does not exist', function() { -// var comp = new Component(getFakePlayer()); - -// throws(function() { -// comp.addChild('non-existent-child'); -// }, new Error('Component Non-existent-child does not exist'), 'addChild threw'); + throws(function() { + comp.addChild('non-existent-child'); + }, new Error('Component Non-existent-child does not exist'), 'addChild threw'); }); + test('should init child components from options', function(){ var comp = new Component(getFakePlayer(), { children: { From 04fef8305657d16d50d265dd0b3fdf636070af72 Mon Sep 17 00:00:00 2001 From: Erik Yuzwa Date: Wed, 3 Feb 2016 00:39:44 -0700 Subject: [PATCH 10/14] modified vars to lets --- src/js/component.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/js/component.js b/src/js/component.js index e664a88b9c..d821ae0244 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -406,8 +406,8 @@ class Component { // Add the UI object's element to the container div (box) // Having an element is not required if (typeof component.el === 'function' && component.el()) { - var childNodes = this.contentEl().children; - var refNode = childNodes[index] || undefined; + let childNodes = this.contentEl().children; + let refNode = childNodes[index] || null; this.contentEl().insertBefore(component.el(), refNode); } From 6efa25e7bf8e82e0ea62ea607b307d7773295403 Mon Sep 17 00:00:00 2001 From: Erik Yuzwa Date: Wed, 3 Feb 2016 00:43:42 -0700 Subject: [PATCH 11/14] undo the DOM el function append --- src/js/player.js | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/js/player.js b/src/js/player.js index 851c227b5a..7c17fc0de2 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -301,16 +301,7 @@ class Player extends Component { tag.parentNode.insertBefore(el, tag); } - //Dom.insertElFirst(tag, el); // Breaks iPhone, fixed in HTML5 setup. - - // append the DOM el function to make use of our component.addChild method - // to properly be injected into the children Array - tag.el = function () { - return this; - }; - - // add our tag as the first child - this.addChild(tag, {}, 0); + Dom.insertElFirst(tag, el); // Breaks iPhone, fixed in HTML5 setup. this.el_ = el; From 266097aa86c03ccb8613b7d5039cd918e64f50e7 Mon Sep 17 00:00:00 2001 From: Erik Yuzwa Date: Wed, 3 Feb 2016 09:11:00 -0700 Subject: [PATCH 12/14] Revert "undo the DOM el function append" This reverts commit 6efa25e7bf8e82e0ea62ea607b307d7773295403. --- src/js/player.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/js/player.js b/src/js/player.js index 7c17fc0de2..851c227b5a 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -301,7 +301,16 @@ class Player extends Component { tag.parentNode.insertBefore(el, tag); } - Dom.insertElFirst(tag, el); // Breaks iPhone, fixed in HTML5 setup. + //Dom.insertElFirst(tag, el); // Breaks iPhone, fixed in HTML5 setup. + + // append the DOM el function to make use of our component.addChild method + // to properly be injected into the children Array + tag.el = function () { + return this; + }; + + // add our tag as the first child + this.addChild(tag, {}, 0); this.el_ = el; From cb492005e6242900e2409473b0c5b014c6ae92ac Mon Sep 17 00:00:00 2001 From: Erik Yuzwa Date: Wed, 3 Feb 2016 13:43:42 -0700 Subject: [PATCH 13/14] updated comments to help clarify dom insertion --- src/js/player.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/js/player.js b/src/js/player.js index 851c227b5a..50d686c356 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -301,6 +301,7 @@ class Player extends Component { tag.parentNode.insertBefore(el, tag); } + // using insertElFirst will cause the video element to move to the last child of the player div //Dom.insertElFirst(tag, el); // Breaks iPhone, fixed in HTML5 setup. // append the DOM el function to make use of our component.addChild method @@ -309,7 +310,7 @@ class Player extends Component { return this; }; - // add our tag as the first child + // use our own method to ensure our tag is the first child this.addChild(tag, {}, 0); this.el_ = el; From 7b7ca3dfc00f4e1e38e9daa2e8da3019800a2221 Mon Sep 17 00:00:00 2001 From: Erik Yuzwa Date: Wed, 3 Feb 2016 14:47:54 -0700 Subject: [PATCH 14/14] reworked dom insertion to allow proper children --- src/js/player.js | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/js/player.js b/src/js/player.js index 50d686c356..b15270e7d8 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -301,17 +301,11 @@ class Player extends Component { tag.parentNode.insertBefore(el, tag); } - // using insertElFirst will cause the video element to move to the last child of the player div - //Dom.insertElFirst(tag, el); // Breaks iPhone, fixed in HTML5 setup. - - // append the DOM el function to make use of our component.addChild method - // to properly be injected into the children Array - tag.el = function () { - return this; - }; - - // use our own method to ensure our tag is the first child - this.addChild(tag, {}, 0); + // insert the tag as the first child of the player element + // then manually add it to the children array so that this.addChild + // will work properly for other components + Dom.insertElFirst(tag, el); // Breaks iPhone, fixed in HTML5 setup. + this.children_.unshift(tag); this.el_ = el;