Skip to content

Commit

Permalink
Major cleanup
Browse files Browse the repository at this point in the history
- Rewrote a lot of the Scrollable logic so that it's more performant. No
need for the throttling.
- Moved all the scrollable logic into one behavior instead of scattered.
- Rewrote large chunks of slidingRender to be more performant and fix a
few trailing bugs.
  • Loading branch information
MeoMix committed Apr 27, 2015
1 parent 300fa82 commit 4debdf7
Show file tree
Hide file tree
Showing 18 changed files with 267 additions and 229 deletions.
1 change: 1 addition & 0 deletions Streamus Chrome Extension.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
<Content Include="src\img\streamus_icon_white19.png" />
<Content Include="src\js\background\model\analyticsManager.js" />
<Content Include="src\js\common\enum\desktopNotificationDuration.js" />
<Content Include="src\js\foreground\view\element\simpleMenuItemsView.js" />
<Content Include="src\js\foreground\view\leftPane\activePlaylistAreaView.js" />
<Content Include="src\js\foreground\view\leftPane\leftPaneRegion.js" />
<Content Include="src\js\foreground\view\leftPane\leftPaneView.js" />
Expand Down
46 changes: 25 additions & 21 deletions src/js/background/plugins.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,35 @@

require('backbone.marionette');
require('backbone.localStorage');
var Cocktail = require('cocktail');

var Cocktail = require('cocktail');
Cocktail.patch(Backbone);

// TODO: I really feel like I should be able to mix this in from the background page, but it throws errors. Need to debug at some point. Probably very scary..
//var lodashMixin = require('common/lodashMixin');
//_.mixin(lodashMixin);

// Some sensitive data is not committed to GitHub. Use an example file to help others and provide detection of incomplete setup.
//requirejs.onError = function(error) {
// var headerWritten = false;
// console.error('error:', error);

// error.requireModules.forEach(function(requireModule) {
// if (requireModule.indexOf('background/key/') !== -1) {
// if (!headerWritten) {
// console.warn('%c ATTENTION! Additional configuration is required', 'color: rgb(66,133,244); font-size: 18px; font-weight: bold;');
// console.warn('%c -----------------------------------------------', 'color: rgb(66,133,244); font-size: 18px; font-weight: bold;');
// headerWritten = true;
// }

// console.warn('%cKey not found. \n Rename "' + requireModule + '.js.example" to "' + requireModule + '.js".\n Then, follow the instructions in the file.', 'color: red');
// }
// });

// if (headerWritten) {
// console.warn('%c -----------------------------------------------', 'color: rgb(66,133,244); font-size: 18px; font-weight: bold;');
// }
//};
requirejs.onError = function(error) {
var headerWritten = false;
console.error('error:', error);

error.requireModules.forEach(function(requireModule) {
if (requireModule.indexOf('background/key/') !== -1) {
if (!headerWritten) {
console.warn('%c ATTENTION! Additional configuration is required', 'color: rgb(66,133,244); font-size: 18px; font-weight: bold;');
console.warn('%c -----------------------------------------------', 'color: rgb(66,133,244); font-size: 18px; font-weight: bold;');
headerWritten = true;
}

console.warn('%cKey not found. \n Rename "' + requireModule + '.js.example" to "' + requireModule + '.js".\n Then, follow the instructions in the file.', 'color: red');
}
});

if (headerWritten) {
console.warn('%c -----------------------------------------------', 'color: rgb(66,133,244); font-size: 18px; font-weight: bold;');
}
};

// Finally, load the application:
require(['background/application']);
Expand Down
17 changes: 17 additions & 0 deletions src/js/common/lodashMixin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
define({
// Inspired by: https://gist.github.com/danro/7846358
// Leverage requestAnimationFrame for throttling function calls instead of setTimeout for better perf.
throttleFramerate: function(callback) {
var wait, args, context;
return function() {
if (wait) return;
wait = true;
args = arguments;
context = this;
requestAnimationFrame(function() {
wait = false;
callback.apply(context, args);
});
};
}
});
7 changes: 7 additions & 0 deletions src/js/foreground/plugins.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
define(function(require) {
'use strict';

// Overwrite _ not only in the AMD module wrapper but also globally here so that 100% of references point to the background page.
window._ = chrome.extension.getBackgroundPage()._;

// TODO: I really feel like I should be able to mix this in from the background page, but it throws errors. Need to debug at some point. Probably very scary..
var lodashMixin = require('common/lodashMixin');
_.mixin(lodashMixin);

require('backbone.marionette');
require('backbone.localStorage');
require('jquery.perfectScrollbar');
Expand Down
72 changes: 26 additions & 46 deletions src/js/foreground/view/behavior/scrollable.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,69 +2,49 @@
'use strict';

var Scrollable = Marionette.Behavior.extend({
collectionEvents: {
// IMPORTANT: These method names are valid in Behavior but NOT in CompositeView or CollectionView; clashes with _onCollectionAdd and _onCollectionRemove in Marionette.
'add:completed': '_onCollectionAddCompleted',
'reset': '_onCollectionReset',
'remove': '_onCollectionRemove'
},

initialize: function() {
// Throttle updating the scrollbar because many events can come in at once, but only need to reflect
// the final state / potentially small updates while events coming in.
// Bound in initialize because if bound on the class then all views which implement Scrollable will
// share the throttle timer.
this._throttleUpdateScrollbar = _.throttle(this._updateScrollbar.bind(this), 20);
defaults: {
// The SlidingRender behavior modifies CollectionView functionality drastically.
// It will trigger OnUpdateScrollbar events when it needs the scrollbar updated.
implementsSlidingRender: false
},

onAttach: function() {
window.requestAnimationFrame(function() {
// More info: https://github.com/noraesae/perfect-scrollbar
// This needs to be ran during onAttach for perfectScrollbar to do its math properly.
this.$el.perfectScrollbar({
suppressScrollX: true,
// 56px because that is the height of 1 listItem--medium
minScrollbarLength: 56,
includePadding: true
});

// When showing a SlidingRender collection which has an initial set of items,
// need to call update after setting up perfectScrollbar to ensure that initial load of items is parsed.
_.defer(this._throttleUpdateScrollbar.bind(this));
}.bind(this));
// More info: https://github.com/noraesae/perfect-scrollbar
// This needs to be ran during onAttach for perfectScrollbar to do its math properly.
this.$el.perfectScrollbar({
suppressScrollX: true,
// 48px because that is the min hit target for a button suggested by Google's design docs.
// So, I figure it's also an ideal minimum size for clicking on a scrollbar.
minScrollbarLength: 48,
includePadding: true
});
},

onUpdateScrollbar: function() {
this._throttleUpdateScrollbar();
this._updateScrollbar();
},

_onCollectionAddCompleted: function() {
this._throttleUpdateScrollbar();
// TODO: This wouldn't be necessary (and bad) if I calculate the height of the view before sliding it out and use transforms.
onListHeightUpdated: function() {
requestAnimationFrame(this._updateScrollbar.bind(this));
},

_onCollectionReset: function() {
this._throttleUpdateScrollbar();
onAddChild: function() {
if (!this.options.implementsSlidingRender) {
this._updateScrollbar();
}
},

_onCollectionRemove: function() {
// TODO: This is poor design. How can I fix this?
// This needs to be deferred because SlidingRender's onCollectionRemove logic is deferred.
_.defer(this._throttleUpdateScrollbar.bind(this));
onRemoveChild: function() {
if (!this.options.implementsSlidingRender) {
this._updateScrollbar();
}
},

_throttleUpdateScrollbar: null,

_updateScrollbar: function() {
// When the CollectionView is first initializing _updateScrollbar can fire before perfectScrollbar has been initialized.
if (this.view._isShown) {
// This needs to be deferred because _onCollection events fire before the view is added or removed.
// Don't bind to onChildAdd/onChildRemove because SlidingRender behavior doesn't run onChildAdd
// once an ItemView exceeds the threshold, but the scrollbar height still needs to be updated.
// Additionally, SlidingRender fires onChildAdd events when scrolling which causes lag when
// the scrollbar does not need to be updated.
_.defer(function() {
this.$el.perfectScrollbar('update');
}.bind(this));
this.$el.perfectScrollbar('update');
}
}
});
Expand Down

0 comments on commit 4debdf7

Please sign in to comment.