Refactored fixedMenuItem and fixed linting

MeoMix · May 16, 2015 · 712013a · 712013a
1 parent aeaf71d
commit 712013a
Show file tree

Hide file tree

Showing 15 changed files with 99 additions and 50 deletions.
diff --git a/src/js/foreground/model/fixedMenuItem.js b/src/js/foreground/model/fixedMenuItem.js
@@ -0,0 +1,11 @@
+define(function() {
+    'use strict';
+
+    var FixedMenuItem = Backbone.Model.extend({
+        defaults: {
+            text: ''
+        }
+    });
+
+    return FixedMenuItem;
+});
diff --git a/src/js/foreground/model/simpleMenu.js b/src/js/foreground/model/simpleMenu.js
@@ -3,7 +3,9 @@
 
     var SimpleMenu = Backbone.Model.extend({
         defaults: {
-            fixedMenuItemTitle: ''
+            simpleMenuItems: null,
+            fixedMenuItem: null,
+            listItemHeight: 0
         }
     });
 

diff --git a/src/js/foreground/view/element/fixedMenuItemView.js b/src/js/foreground/view/element/fixedMenuItemView.js
@@ -0,0 +1,16 @@
+define(function(require) {
+    'use strict';
+
+    var MenuItemTemplate = require('text!template/element/menuItem.html');
+
+    var FixedMenuItemView = Marionette.ItemView.extend({
+        className: 'listItem listItem--small listItem--clickable listItem--selectable u-bordered--top',
+        template: _.template(MenuItemTemplate),
+
+        triggers: {
+            'click': 'click:fixedMenuItem'
+        }
+    });
+
+    return FixedMenuItemView;
+});
diff --git a/src/js/foreground/view/element/simpleListItemView.js b/src/js/foreground/view/element/simpleListItemView.js
@@ -68,16 +68,17 @@
                 //  otherwise it'll close immediately.
                 _.defer(function() {
                     this.showChildView('simpleMenu', new SimpleMenuView({
-                        simpleMenuItems: simpleMenuItems,
-                        model: new SimpleMenu(),
-                        listItemHeight: this.$el.height()
+                        model: new SimpleMenu({
+                            simpleMenuItems: simpleMenuItems,
+                            listItemHeight: this.$el.height()
+                        })
                     }));
                 }.bind(this));
             }
         },
 
         _onClickSimpleMenuItem: function(model, eventArgs) {
-            var activeItem = eventArgs.collection.getActive();
+            var activeItem = eventArgs.model.get('simpleMenuItems').getActive();
             this.model.set('value', activeItem.get('value'));
         }
     });

diff --git a/src/js/foreground/view/element/simpleMenuItemView.js b/src/js/foreground/view/element/simpleMenuItemView.js
@@ -1,11 +1,11 @@
 define(function(require) {
     'use strict';
 
-    var SimpleMenuItemTemplate = require('text!template/element/simpleMenuItem.html');
+    var MenuItemTemplate = require('text!template/element/menuItem.html');
 
     var SimpleMenuItemView = Marionette.LayoutView.extend({
         className: 'listItem listItem--small listItem--selectable listItem--clickable',
-        template: _.template(SimpleMenuItemTemplate),
+        template: _.template(MenuItemTemplate),
 
         events: {
             'click': '_onClick'

diff --git a/src/js/foreground/view/element/simpleMenuView.js b/src/js/foreground/view/element/simpleMenuView.js
@@ -2,6 +2,7 @@
     'use strict';
 
     var SimpleMenuItemsView = require('foreground/view/element/simpleMenuItemsView');
+    var FixedMenuItemView = require('foreground/view/element/fixedMenuItemView');
     var SimpleMenuTemplate = require('text!template/element/simpleMenu.html');
 
     var SimpleMenuView = Marionette.LayoutView.extend({
@@ -10,45 +11,43 @@
         template: _.template(SimpleMenuTemplate),
 
         regions: {
-            simpleMenuItems: '[data-region=simpleMenuItems]'
+            simpleMenuItems: '[data-region=simpleMenuItems]',
+            fixedMenuItem: '[data-region=fixedMenuItem]'
         },
 
         ui: {
-            //  TODO: Fix this so that fixedMenuItem can be rendered or use a region or something. Need to pass my ui testing.
-            fixedMenuItem: '[data-ui~=fixedMenuItem]',
             panelContent: '[data-ui~=panelContent]'
         },
 
-        events: {
-            'click @ui.fixedMenuItem': '_onClickFixedMenuItem'
-        },
-
         childEvents: {
-            'click:simpleMenuItem': '_onClickSimpleMenuItem'
+            'click:simpleMenuItem': '_onClickSimpleMenuItem',
+            'click:fixedMenuItem': '_onClickFixedMenuItem'
         },
 
-        listItemHeight: 0,
-        simpleMenuItems: null,
-
-        initialize: function(options) {
-            this.simpleMenuItems = options.simpleMenuItems;
-            this.listItemHeight = _.isUndefined(options.listItemHeight) ? this.listItemHeight : options.listItemHeight;
+        initialize: function() {
             this.listenTo(Streamus.channels.element.vent, 'click', this._onElementClick);
         },
 
         onRender: function() {
             this.showChildView('simpleMenuItems', new SimpleMenuItemsView({
-                collection: this.simpleMenuItems,
-                listItemHeight: this.listItemHeight
+                collection: this.model.get('simpleMenuItems'),
+                listItemHeight: this.model.get('listItemHeight')
             }));
+
+            if (this.model.has('fixedMenuItem')) {
+                this.showChildView('fixedMenuItem', new FixedMenuItemView({
+                    model: this.model.get('fixedMenuItem')
+                }));
+            }
         },
 
         onAttach: function() {
             //  This needs to be ran on the parent because _centerActive has a dependency on simpleMenuItems scrollTop which is set here.
             this.getChildView('simpleMenuItems').ensureActiveIsVisible();
 
-            if (this.listItemHeight > 0) {
-                this._centerActive();
+            var listItemHeight = this.model.get('listItemHeight');
+            if (listItemHeight > 0) {
+                this._centerActive(listItemHeight);
             }
 
             requestAnimationFrame(function() {
@@ -65,8 +64,7 @@
         _onClickSimpleMenuItem: function() {
             this.triggerMethod('click:simpleMenuItem', {
                 view: this,
-                model: this.model,
-                collection: this.simpleMenuItems
+                model: this.model
             });
             Streamus.channels.simpleMenu.vent.trigger('clicked:item');
             this.hide();
@@ -75,8 +73,7 @@
         _onClickFixedMenuItem: function() {
             this.triggerMethod('click:fixedMenuItem', {
                 view: this,
-                model: this.model,
-                collection: this.simpleMenuItems
+                model: this.model
             });
             Streamus.channels.simpleMenu.vent.trigger('clicked:item');
             this.hide();
@@ -95,9 +92,10 @@
 
         //  TODO: This should also take into account overflow. If overflow would happen, abandon trying to perfectly center and keep the menu within the viewport.
         //  When showing this view over a ListItem, center the view's active item over the ListItem.
-        _centerActive: function() {
+        _centerActive: function(listItemHeight) {
+            var simpleMenuItems = this.model.get('simpleMenuItems');
             //  Adjust the top position of the view based on which item is active.
-            var index = this.simpleMenuItems.indexOf(this.simpleMenuItems.getActive());
+            var index = simpleMenuItems.indexOf(simpleMenuItems.getActive());
 
             //  TODO: This feels weirdly coupled.
             var childHeight = this.getChildView('simpleMenuItems').children.first().$el.height();
@@ -109,7 +107,7 @@
 
             //  Now center the item over its ListItem
             var paddingTop = parseInt(this.ui.panelContent.css('padding-top'));
-            var centering = (this.listItemHeight - childHeight - paddingTop) / 2;
+            var centering = (listItemHeight - childHeight - paddingTop) / 2;
 
             this.$el.css('top', offset + centering);
         }

diff --git a/src/js/foreground/view/saveSongs/saveSongsRegion.js b/src/js/foreground/view/saveSongs/saveSongsRegion.js
@@ -3,6 +3,7 @@ define(function(require) {
     'use strict';
 
     var SimpleMenuItems = require('foreground/collection/simpleMenuItems');
+    var FixedMenuItem = require('foreground/model/fixedMenuItem');
     var SimpleMenu = require('foreground/model/simpleMenu');
     var SimpleMenuView = require('foreground/view/element/simpleMenuView');
     var CreatePlaylistDialogView = require('foreground/view/dialog/createPlaylistDialogView');
@@ -30,9 +31,11 @@ define(function(require) {
                 }));
 
                 var simpleMenuView = new SimpleMenuView({
-                    simpleMenuItems: simpleMenuItems,
                     model: new SimpleMenu({
-                        fixedMenuItemTitle: chrome.i18n.getMessage('createPlaylist')
+                        simpleMenuItems: simpleMenuItems, 
+                        fixedMenuItem: new FixedMenuItem({
+                            text: chrome.i18n.getMessage('createPlaylist')
+                        })
                     })
                 });
 
@@ -53,7 +56,7 @@ define(function(require) {
         },
 
         _onClickSimpleMenuItem: function(playlists, songs, eventArgs) {
-            var activeItem = eventArgs.collection.getActive();
+            var activeItem = eventArgs.model.get('simpleMenuItems').getActive();
             var playlist = playlists.get(activeItem.get('value'));
             playlist.get('items').addSongs(songs);
         },

diff --git a/src/js/test/foreground/view/appBar/nextButtonView.spec.js b/src/js/test/foreground/view/appBar/nextButtonView.spec.js
@@ -24,7 +24,7 @@
                     stream: new Stream({
                         player: new Player({
                             settings: new Settings(),
-                            youTubePlayer: new YouTubePlayer
+                            youTubePlayer: new YouTubePlayer()
                         }),
                         shuffleButton: shuffleButton,
                         radioButton: radioButton,

diff --git a/src/js/test/foreground/view/appBar/playPauseButtonView.spec.js b/src/js/test/foreground/view/appBar/playPauseButtonView.spec.js
@@ -14,7 +14,7 @@
 
             var player = new Player({
                 settings: new Settings(),
-                youTubePlayer: new YouTubePlayer
+                youTubePlayer: new YouTubePlayer()
             });
 
             this.playPauseButtonView = new PlayPauseButtonView({

diff --git a/src/js/test/foreground/view/appBar/previousButtonView.spec.js b/src/js/test/foreground/view/appBar/previousButtonView.spec.js
@@ -17,7 +17,7 @@
 
             var player = new Player({
                 settings: new Settings(),
-                youTubePlayer: new YouTubePlayer
+                youTubePlayer: new YouTubePlayer()
             });
 
             var shuffleButton = new ShuffleButton();

diff --git a/src/js/test/foreground/view/element/elementSpecLoader.js b/src/js/test/foreground/view/element/elementSpecLoader.js
@@ -2,6 +2,7 @@
     'use strict';
 
     require('test/foreground/view/element/checkboxView.spec');
+    require('test/foreground/view/element/fixedMenuItemView.spec');
     require('test/foreground/view/element/radioButtonView.spec');
     require('test/foreground/view/element/radioGroupView.spec');
     require('test/foreground/view/element/simpleListItemView.spec');

diff --git a/src/js/test/foreground/view/element/fixedMenuItemView.spec.js b/src/js/test/foreground/view/element/fixedMenuItemView.spec.js
@@ -0,0 +1,27 @@
+define(function(require) {
+    'use strict';
+
+    var FixedMenuItemView = require('foreground/view/element/fixedMenuItemView');
+    var FixedMenuItem = require('foreground/model/fixedMenuItem');
+
+    describe('FixedMenuItemView', function() {
+        beforeEach(function() {
+            this.documentFragment = document.createDocumentFragment();
+            this.fixedMenuItemView = new FixedMenuItemView({
+                model: new FixedMenuItem()
+            });
+        });
+
+        afterEach(function() {
+            this.fixedMenuItemView.destroy();
+        });
+
+        it('should be able to find all referenced ui targets', function() {
+            this.documentFragment.appendChild(this.fixedMenuItemView.render().el);
+
+            _.forIn(this.fixedMenuItemView.ui, function(element) {
+                expect(element.length).to.not.equal(0);
+            });
+        });
+    });
+});
diff --git a/src/js/test/foreground/view/element/simpleMenuView.spec.js b/src/js/test/foreground/view/element/simpleMenuView.spec.js
@@ -16,8 +16,7 @@
             this.simpleMenuView.destroy();
         });
 
-        //  TODO: fix
-        xit('should be able to find all referenced ui targets', function() {
+        it('should be able to find all referenced ui targets', function() {
             this.documentFragment.appendChild(this.simpleMenuView.render().el);
 
             _.forIn(this.simpleMenuView.ui, function(element) {

diff --git a/src/template/element/simpleMenuItem.html → src/template/element/menuItem.html b/src/template/element/simpleMenuItem.html → src/template/element/menuItem.html
diff --git a/src/template/element/simpleMenu.html b/src/template/element/simpleMenu.html
@@ -1,13 +1,4 @@
 <div data-ui='panelContent' class='menu-items panel-content panel-content--uncolored panel-content--fadeInOut'>
     <div data-region='simpleMenuItems'></div>
-
-    <% if(fixedMenuItemTitle !== '') { %>
-        <div data-ui='fixedMenuItem' class='listItem listItem--small listItem--clickable listItem--selectable u-bordered--top'>
-            <div class='listItem-content'>
-                <div class='listItem-title'>
-                    <%- fixedMenuItemTitle %>
-                </div>
-            </div>
-        </div>
-    <% } %>
+    <div data-region='fixedMenuItem'></div>
 </div>