Skip to content

Commit

Permalink
Refactoring to pass dependencies into views for testability
Browse files Browse the repository at this point in the history
  • Loading branch information
MeoMix committed May 14, 2015
1 parent 5e5b004 commit 0421d1e
Show file tree
Hide file tree
Showing 39 changed files with 135 additions and 89 deletions.
9 changes: 9 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
This is a complete work in progress. Right now I'm just putting coding standards in as I think of them.


Coding Guidelines:

Behaviors/Mixins:

- Prefer using behaviors and mixins over prototypical inheritance. It is less error prone and more flexible.
- Mixins and behaviors should be named as an adjective/descriptor. They are 'giving' a state to another entity. 'Tooltipable, Hoverable, Sortable' etc.
16 changes: 1 addition & 15 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,18 +81,4 @@ Streamus uses a fair number of third-party JavaScript libraries. Introduction of

* [MeoMix](https://github.com/MeoMix)

with translation support provided by a community of volunteers.

<h2>License</h2>

Licensed under the Apache License, Version 2.0 (the "License");
you may not use any files in this repository except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
with translation support provided by a community of volunteers.
1 change: 1 addition & 0 deletions Streamus Chrome Extension.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@
<Content Include="LICENSE" />
<Content Include="src\js\background\key\youTubeAPIKey.js.example" />
<Content Include="changelog.md" />
<Content Include="CONTRIBUTING.md" />
<None Include="src\js\thirdParty\chrome-api-vsdoc.js" />
<Content Include="src\less\color.less" />
<Content Include="src\less\font.less" />
Expand Down
6 changes: 5 additions & 1 deletion src/js/foreground/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@
},

_showForegroundArea: function() {
var foregroundAreaView = new ForegroundAreaView();
var foregroundAreaView = new ForegroundAreaView({
player: Streamus.backgroundPage.player,
settings: Streamus.backgroundPage.settings,
analyticsManager: Streamus.backgroundPage.analyticsManager
});
foregroundAreaView.render();
}
});
Expand Down
4 changes: 2 additions & 2 deletions src/js/foreground/view/appBar/adminMenuAreaView.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@
'click': '_onElementClick'
},

initialize: function() {
this.tabManager = Streamus.backgroundPage.tabManager;
initialize: function(options) {
this.tabManager = options.tabManager;
this.bindEntityEvents(Streamus.channels.element.vent, this.elementEvents);
},

Expand Down
5 changes: 4 additions & 1 deletion src/js/foreground/view/appBar/appBarRegion.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
},

_onForegroundAreaRendered: function() {
this.show(new AppBarView());
this.show(new AppBarView({
signInManager: Streamus.backgroundPage.signInManager,
search: Streamus.backgroundPage.search
}));
}
});

Expand Down
16 changes: 10 additions & 6 deletions src/js/foreground/view/appBar/appBarView.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@
signInManager: null,
search: null,

initialize: function() {
this.signInManager = Streamus.backgroundPage.signInManager;
this.search = Streamus.backgroundPage.search;
initialize: function(options) {
this.signInManager = options.signInManager;
this.search = options.search;

this.listenTo(this.signInManager, 'change:signedInUser', this._onSignInManagerChangeSignedInUser);
this.listenTo(this.search, 'change:query', this._onSearchChangeQuery);
Expand All @@ -99,18 +99,22 @@
this._setPlaylistTitleRegion(signedInUser);
}

this.showChildView('volumeAreaRegion', new VolumeAreaView());
this.showChildView('volumeAreaRegion', new VolumeAreaView({
player: Streamus.backgroundPage.player
}));

this.showChildView('adminMenuAreaRegion', new AdminMenuAreaView({
model: new AdminMenuArea()
model: new AdminMenuArea(),
tabManager: Streamus.backgroundPage.tabManager
}));

this.showChildView('previousButtonRegion', new PreviousButtonView({
model: Streamus.backgroundPage.previousButton
}));

this.showChildView('playPauseButtonRegion', new PlayPauseButtonView({
model: Streamus.backgroundPage.playPauseButton
model: Streamus.backgroundPage.playPauseButton,
player: Streamus.backgroundPage.player
}));

this.showChildView('nextButtonRegion', new NextButtonView({
Expand Down
4 changes: 2 additions & 2 deletions src/js/foreground/view/appBar/playPauseButtonView.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@

player: null,

initialize: function() {
this.player = Streamus.backgroundPage.player;
initialize: function(options) {
this.player = options.player;
this.listenTo(this.player, 'change:state', this._onPlayerChangeState);

this.listenTo(Streamus.channels.playPauseButton.commands, 'tryToggle:playerState', this._tryTogglePlayerState);
Expand Down
4 changes: 2 additions & 2 deletions src/js/foreground/view/appBar/volumeAreaView.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ define(function(require) {

player: null,

initialize: function() {
this.player = Streamus.backgroundPage.player;
initialize: function(options) {
this.player = options.player;

this.listenTo(this.player, 'change:muted', this._onPlayerChangeMuted);
this.listenTo(this.player, 'change:volume', this._onPlayerChangeVolume);
Expand Down
1 change: 1 addition & 0 deletions src/js/foreground/view/dialog/aboutStreamusDialogView.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
this.model = new Dialog();

this.contentView = new AboutStreamusView({
tabManager: Streamus.backgroundPage.tabManager
});

DialogView.prototype.initialize.apply(this, arguments);
Expand Down
4 changes: 2 additions & 2 deletions src/js/foreground/view/dialog/aboutStreamusView.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@

tabManager: null,

initialize: function() {
this.tabManager = Streamus.backgroundPage.tabManager;
initialize: function(options) {
this.tabManager = options.tabManager;
},

_onClickOpenHomepage: function() {
Expand Down
2 changes: 2 additions & 0 deletions src/js/foreground/view/dialog/createPlaylistDialogView.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
});

this.contentView = new CreatePlaylistView({
dataSourceManager: Streamus.backgroundPage.dataSourceManager,
playlists: Streamus.backgroundPage.signInManager.get('signedInUser').get('playlists'),
songs: options && options.songs ? options.songs : []
});

Expand Down
6 changes: 3 additions & 3 deletions src/js/foreground/view/dialog/createPlaylistView.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@
songs: [],

initialize: function(options) {
this.songs = options && options.songs ? options.songs : this.songs;
this.playlists = Streamus.backgroundPage.signInManager.get('signedInUser').get('playlists');
this.dataSourceManager = Streamus.backgroundPage.dataSourceManager;
this.playlists = options.playlists;
this.dataSourceManager = options.dataSourceManager;
this.songs = _.isUndefined(options.songs) ? this.songs : options.songs;
},

onRender: function() {
Expand Down
6 changes: 3 additions & 3 deletions src/js/foreground/view/dialog/dialogRegion.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
player: null,
signInManager: null,

initialize: function() {
this.player = Streamus.backgroundPage.player;
this.signInManager = Streamus.backgroundPage.signInManager;
initialize: function(options) {
this.player = options.player;
this.signInManager = options.signInManager;

this.listenTo(Streamus.channels.dialog.commands, 'show:dialog', this._showDialog);
this.listenTo(Streamus.channels.foregroundArea.vent, 'idle', this._onForegroundAreaIdle);
Expand Down
3 changes: 2 additions & 1 deletion src/js/foreground/view/dialog/settingsDialogView.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
});

this.contentView = new SettingsView({
model: Streamus.backgroundPage.settings
model: Streamus.backgroundPage.settings,
signInManager: Streamus.backgroundPage.signInManager
});

DialogView.prototype.initialize.apply(this, arguments);
Expand Down
6 changes: 4 additions & 2 deletions src/js/foreground/view/dialog/settingsView.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@
simpleListItems: null,
signInManager: null,

initialize: function() {
initialize: function(options) {
this.checkboxes = new Checkboxes();
this.radioGroups = new RadioGroups();
this.switches = new Switches();
this.simpleListItems = new SimpleListItems();

this.signInManager = Streamus.backgroundPage.signInManager;
this.signInManager = options.signInManager;
},

onRender: function() {
Expand Down Expand Up @@ -96,6 +96,8 @@
options: options.options
});

console.log('simpleListItem:', simpleListItem);

this[propertyName + 'Region'].show(new SimpleListItemView({
model: simpleListItem
}));
Expand Down
2 changes: 1 addition & 1 deletion src/js/foreground/view/element/simpleListItemView.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

templateHelpers: function() {
return {
'data-tooltip-text': chrome.i18n.getMessage(this.model.get('labelKey'))
title: chrome.i18n.getMessage(this.model.get('labelKey'))
};
},

Expand Down
26 changes: 17 additions & 9 deletions src/js/foreground/view/foregroundAreaView.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@
},
dialogRegion: {
selector: '#' + this.id + '-dialogRegion',
regionClass: DialogRegion
regionClass: DialogRegion,
player: Streamus.backgroundPage.player,
signInManager: Streamus.backgroundPage.signInManager
},
notificationRegion: {
selector: '#' + this.id + '-notificationRegion',
Expand All @@ -69,19 +71,23 @@
},
leftPaneRegion: {
selector: '#' + this.id + '-leftPaneRegion',
regionClass: LeftPaneRegion
regionClass: LeftPaneRegion,
settings: Streamus.backgroundPage.settings
},
searchAreaRegion: {
selector: '#' + this.id + '-searchAreaRegion',
regionClass: SearchAreaRegion
regionClass: SearchAreaRegion,
settings: Streamus.backgroundPage.settings
},
saveSongsRegion: {
selector: '#' + this.id + '-saveSongsRegion',
regionClass: SaveSongsRegion
regionClass: SaveSongsRegion,
signInManager: Streamus.backgroundPage.signInManager
},
playlistsAreaRegion: {
selector: '#' + this.id + '-playlistsAreaRegion',
regionClass: PlaylistsAreaRegion
regionClass: PlaylistsAreaRegion,
signInManager: Streamus.backgroundPage.signInManager
},
streamRegion: {
selector: '#' + this.id + '-streamRegion',
Expand All @@ -104,15 +110,17 @@

player: null,
settings: null,
analyticsManager: null,

playerEvents: {
'change:loading': '_onPlayerChangeLoading',
'change:currentLoadAttempt': '_onPlayerChangeCurrentLoadAttempt'
},

initialize: function() {
this.player = Streamus.backgroundPage.player;
this.settings = Streamus.backgroundPage.settings;
initialize: function(options) {
this.player = options.player;
this.settings = options.settings;
this.analyticsManager = options.analyticsManager;
this.bindEntityEvents(this.player, this.playerEvents);

// It's important to bind pre-emptively or attempts to call removeEventListener will fail to find the appropriate reference.
Expand All @@ -126,7 +134,7 @@
window.addEventListener('error', this._onWindowError);
window.addEventListener('keydown', this._onKeyDown);

Streamus.backgroundPage.analyticsManager.sendPageView('/foreground.html');
this.analyticsManager.sendPageView('/foreground.html');
},

onRender: function() {
Expand Down
4 changes: 2 additions & 2 deletions src/js/foreground/view/leftPane/activePlaylistAreaView.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@
'reset': '_onPlaylistItemsReset'
},

initialize: function() {
this.streamItems = Streamus.backgroundPage.stream.get('items');
initialize: function(options) {
this.streamItems = options.streamItems;
this.bindEntityEvents(this.streamItems, this.streamItemsEvents);

var playlistItems = this.model.get('items');
Expand Down
20 changes: 14 additions & 6 deletions src/js/foreground/view/leftPane/leftPaneRegion.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,31 @@
var LeftPaneRegion = Marionette.Region.extend({
settings: null,

initialize: function() {
this.settings = Streamus.backgroundPage.settings;
initialize: function(options) {
this.settings = options.settings;
this.listenTo(Streamus.channels.foregroundArea.vent, 'rendered', this._onForegroundAreaRendered);
this.listenTo(Streamus.channels.foregroundArea.vent, 'idle', this._onForegroundAreaIdle);
},

_onForegroundAreaRendered: function() {
if (!this.settings.get('openToSearch') && !this._leftPaneViewExists()) {
this.show(new LeftPaneView());
if (!this.settings.get('openToSearch')) {
this._showLeftPaneView();
}
},

_onForegroundAreaIdle: function() {
// If search is being shown immediately then its OK to defer loading to improve initial
// load performance.
if (this.settings.get('openToSearch') && !this._leftPaneViewExists()) {
this.show(new LeftPaneView());
if (this.settings.get('openToSearch')) {
this._showLeftPaneView();
}
},

_showLeftPaneView: function() {
if (!this._leftPaneViewExists()) {
this.show(new LeftPaneView({
signInManager: Streamus.backgroundPage.signInManager
}));
}
},

Expand Down
7 changes: 4 additions & 3 deletions src/js/foreground/view/leftPane/leftPaneView.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@

signInManager: null,

initialize: function() {
this.signInManager = Streamus.backgroundPage.signInManager;
initialize: function(options) {
this.signInManager = options.signInManager;
this.listenTo(this.signInManager, 'change:signedInUser', this._onSignInManagerChangeSignedInUser);

var signedInUser = this.signInManager.get('signedInUser');
Expand Down Expand Up @@ -56,7 +56,8 @@
_showActivePlaylistContent: function(activePlaylist) {
this.showChildView('contentRegion', new ActivePlaylistAreaView({
model: activePlaylist,
collection: activePlaylist.get('items')
collection: activePlaylist.get('items'),
streamItems: Streamus.backgroundPage.stream.get('items')
}));
},

Expand Down
6 changes: 3 additions & 3 deletions src/js/foreground/view/leftPane/playlistItemView.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@
streamItems: null,
player: null,

initialize: function() {
this.streamItems = Streamus.backgroundPage.stream.get('items');
this.player = Streamus.backgroundPage.player;
initialize: function(options) {
this.streamItems = options.streamItems;
this.player = options.player;
},

onRender: function() {
Expand Down
2 changes: 2 additions & 0 deletions src/js/foreground/view/leftPane/playlistItemsView.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
childViewType: ListItemType.PlaylistItem,
childViewOptions: function() {
return {
streamItems: Streamus.backgroundPage.stream.get('items'),
player: Streamus.backgroundPage.player,
type: this.childViewType,
parentId: this.ui.childContainer[0].id
};
Expand Down

0 comments on commit 0421d1e

Please sign in to comment.