From b43ff117921d1f8919c45f97590434da952c8885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Z=C3=A9=20Bateira?= Date: Fri, 23 May 2014 13:56:09 +0100 Subject: [PATCH 1/5] artistmenu docs and refactoring WIP --- js/controllers/artistmenu.js | 94 ++++++++++++++++++++---------------- js/controllers/tagsmenu.js | 2 +- js/controllers/tracklist.js | 4 ++ js/main.js | 4 +- js/models/element.js | 32 ++++++------ 5 files changed, 76 insertions(+), 60 deletions(-) diff --git a/js/controllers/artistmenu.js b/js/controllers/artistmenu.js index b3251ed..993470c 100644 --- a/js/controllers/artistmenu.js +++ b/js/controllers/artistmenu.js @@ -40,6 +40,7 @@ require([ this.bindAllEvents(); }); }, + // Update the UI component given the newArtist parameter updateView: function(newArtist) { // if no parameter has been given or if it's the same artist @@ -112,7 +113,7 @@ require([ // albumsAdded - number of added albums // used to limit the number of albums added. - // sometimes the API returns null albums + // note: sometimes the API returns null albums for (var i = 0, albumsAdded = 0; i <= albumSnapshot.length && albumsAdded < ArtistMenu.MAX_ALBUMS; ++i) { var albumgroup = albumSnapshot.get(i); @@ -124,16 +125,19 @@ require([ width: 50, height: 50, style: 'plain', + link: 'auto', player: true, placeholder: 'album', - link: 'auto', title: album.name }); + // for each album create a DOM element var albumElement = document.createElement('span'); - albumElement.className = 'artist-album artist-album-cover'; + albumElement.className = + 'artist-album artist-album-cover'; albumElement.appendChild(albumImage.node); + // and append it to the albums wrapper jalbums.append(albumElement); albumsAdded++; } @@ -178,56 +182,63 @@ require([ url: url, context: this }).done(function(data) { - this.tags = data.response.terms; - + // the echonest's tags are called terms + this.tags = _.sortBy(data.response.terms + .splice(0, ArtistMenu.MAX_TAGS), 'name'); + // clear the displayed tags this.elements.tags.reset(); - if (this.tags.length > 0) { - this.elements.tagsTitle.html('Tags:
'); - - for (var i = 0, tagsAdded = 0; i < this.tags.length && - tagsAdded < ArtistMenu.MAX_TAGS; ++i) { + if (this.tags.length <= 0) + return; - if (this.tags[i]) { - var tagElement = document.createElement('span'); - tagElement.className = 'artist-tag'; - tagElement.innerHTML = this.tags[i].name; + this.elements.tagsTitle.html('Tags:
'); - this.elements.tags.jelement.append(tagElement); - tagsAdded++; - } - } + for (var i = 0, tagsAdded = 0; i < this.tags.length; ++i) { + // for each tag, create a DOM element + var tagElement = document.createElement('span'); + tagElement.className = 'artist-tag'; + tagElement.innerHTML = this.tags[i].name; - this.artist.tags = this.tags; + // and append it to the tags' wrapper + this.elements.tags.jelement.append(tagElement); } + this.artist.tags = this.tags; + }).fail(function() { // Temporary fix for requests limit from echonest // just... don't show any tags. - $(this.selectors.tagsTitle).html(''); + // Note: since the API key being used has request limits, + // sometimes the limit is reached very easily. If so + // don't show anything. + this.elements.tagsTitle.reset(); }); - }, // Events - bindAllEvents: function() { + // the artistmenu is always in sync with + // the current playing track models.player.addEventListener('change', this.onPlayerChange.bind(this)); + // the artistmenu always updates when a new node + // as been selected this.graphcontroller.addGraphEvent('click', this.onClickNode.bind(this)); - var controls = { - expand: 'onBtnExpandClick', - new: 'onBtnNewClick' - }; - - for (var control in controls) { - document.getElementById('control_' + control) - .onclick = this[controls[control]].bind(this); - } + // Controls' Events + this.elements.controlExpand.addDOMEvent({ + eventName: 'onclick', + handler: this.onBtnExpandClick, + context: this + }); + this.elements.controlNew.addDOMEvent({ + eventName: 'onclick', + handler: this.onBtnNewClick, + context: this + }); }, onClickNode: function(data) { var node = _.findWhere( @@ -239,16 +250,16 @@ require([ return; if (node.id === 1) { - this.jelement.find(this.selectors.controls).hide(); + this.elements.controls.jelement.hide(); } else { - $(this.selectors.control_new).show(); - this.jelement.find(this.selectors.controls).show(); + this.elements.controlNew.jelement.show(); + this.elements.controls.jelement.show(); } if (node.isLeaf) { - this.jelement.find(this.selectors.control_expand).show(); + this.elements.controlExpand.jelement.show(); } else { - this.jelement.find(this.selectors.control_expand).hide(); + this.elements.controlExpand.jelement.hide(); } this.updateView(node.artist); @@ -267,9 +278,9 @@ require([ this.updateView(artist); - this.jelement.find(this.selectors.controls).show(); - this.jelement.find(this.selectors.control_new).show(); - this.jelement.find(this.selectors.control_expand).hide(); + this.elements.controls.jelement.show(); + this.elements.controlNew.jelement.show(); + this.elements.controlExpand.jelement.hide(); }); }, onBtnExpandClick: function(event) { @@ -345,12 +356,11 @@ require([ }); }); - this.jelement.find(this.selectors.control_expand).hide(); + this.elements.controlExpand.jelement.hide(); }, onBtnNewClick: function(event) { this.graphcontroller.setArtistGraph(this.artist); - this.jelement.find(this.selectors.control_new).hide(); - this.jelement.find(this.selectors.control_delete).hide(); + this.elements.controlNew.jelement.hide(); } }); diff --git a/js/controllers/tagsmenu.js b/js/controllers/tagsmenu.js index b844609..6c92890 100644 --- a/js/controllers/tagsmenu.js +++ b/js/controllers/tagsmenu.js @@ -132,7 +132,7 @@ require([ if (!node.tags) { var url = "http://developer.echonest.com/api/v4/artist/" + "terms?api_key=29N71ZBQUW4XN0QXF&" + - "format=json&sort=weight&" + + "format=json&sort=frequency&" + "id=" + node.artist.uri.replace('spotify', 'spotify-WW'); $.ajax({ diff --git a/js/controllers/tracklist.js b/js/controllers/tracklist.js index 4808a15..d6dcc13 100644 --- a/js/controllers/tracklist.js +++ b/js/controllers/tracklist.js @@ -74,6 +74,10 @@ require([ width: 50, height: 50, style: 'plain', + link: 'auto', + player: true, + placeholder: 'artist', + title: artist.name }).node); }, // load the appropriate title into the UI component diff --git a/js/main.js b/js/main.js index ae4b473..4884125 100644 --- a/js/main.js +++ b/js/main.js @@ -136,8 +136,8 @@ spotify.require([ tagsTitle: '#artist_tags_title', tags: '#artist_tags', controls: '#artist_controls', - control_expand: '#control_expand', - control_new: '#control_new' + controlExpand: '#control_expand', + controlNew: '#control_new' }, hasDependencies: true }, diff --git a/js/models/element.js b/js/models/element.js index 2bd371b..6f91d07 100644 --- a/js/models/element.js +++ b/js/models/element.js @@ -8,6 +8,23 @@ var Element = function(selector) { this.element = this.jelement[0]; }; +// Instances only methods +Element.prototype = { + addDOMEvent: function(config) { + this.element[config.eventName] = + config.handler.bind(config.context); + }, + html: function(html) { + this.jelement.html(html); + }, + reset: function() { + this.jelement.html(''); + } +}; + +Element.prototype.constructor = Element; + +// Object only methods Element.createElements = function(selectors) { var elements = {}; @@ -24,21 +41,6 @@ Element.resetAll = function(elements) { }); }; -Element.prototype = { - addDOMEvent: function(config) { - this.element[config.eventName] = - config.handler.bind(config.context); - }, - html: function(html) { - this.jelement.html(html); - }, - reset: function() { - this.jelement.html(''); - } -}; - -Element.prototype.constructor = Element; - require(['$api/models'], function(_models) { exports.element = Element; }); \ No newline at end of file From c2360d85e8f543d309ba62739eb9f6a95d53749c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Z=C3=A9=20Bateira?= Date: Fri, 23 May 2014 15:30:44 +0100 Subject: [PATCH 2/5] artistmenu refactoring almost done. --- bower.json | 2 +- js/controllers/artistmenu.js | 210 +++++++++++++----------------- js/controllers/graphcontroller.js | 14 +- js/models/artistgraph.js | 68 +++++++--- 4 files changed, 151 insertions(+), 143 deletions(-) diff --git a/bower.json b/bower.json index 331b027..75b0249 100644 --- a/bower.json +++ b/bower.json @@ -21,7 +21,7 @@ ], "dependencies": { "jquery": "^2.1.0", - "vis": "^0.7.0", + "vis": "^1.0.0", "normalize-css": "^3.0.0", "underscore": "^1.6.0", "mootools": "^1.4.5" diff --git a/js/controllers/artistmenu.js b/js/controllers/artistmenu.js index 993470c..8bb118e 100644 --- a/js/controllers/artistmenu.js +++ b/js/controllers/artistmenu.js @@ -58,6 +58,8 @@ require([ .done(this, this.updateInfo.bind(this)); this.updateTags(this.artist); + + this.updateControls(this.artist); }, updateImage: function(artist) { @@ -150,7 +152,7 @@ require([ updateTags: function(artist) { // Paul Lamere // http://developer.echonest.com/forums/thread/353 - // Artist terms -> + // Artist terms (tags) -> // what is the difference between weight and frequency // term frequency is directly proportional to how often @@ -182,15 +184,20 @@ require([ url: url, context: this }).done(function(data) { - // the echonest's tags are called terms - this.tags = _.sortBy(data.response.terms - .splice(0, ArtistMenu.MAX_TAGS), 'name'); + // clear the displayed tags this.elements.tags.reset(); + this.elements.tagsTitle.reset(); - if (this.tags.length <= 0) + // not a success response + if (data.response.status.code !== 0 || + (data.response.terms && data.response.terms.length <= 0)) return; + // the echonest's tags are called terms + this.tags = _.sortBy(data.response.terms + .splice(0, ArtistMenu.MAX_TAGS), 'name'); + this.elements.tagsTitle.html('Tags:
'); for (var i = 0, tagsAdded = 0; i < this.tags.length; ++i) { @@ -214,153 +221,114 @@ require([ this.elements.tagsTitle.reset(); }); }, + // control buttons update: shows/hides controls + // based on the artist + updateControls: function(artist) { + var node = _.findWhere( + this.graphcontroller.artistGraph.data.nodes, { + label: artist.name + }); + + // no node found for given artist. + // This means the artist is not on the graph. + if (!node) { + this.elements.controls.jelement.show(); + this.elements.controlNew.jelement.show(); + this.elements.controlExpand.jelement.hide(); + return; + } + + if (node.isRoot) + // if the node is root, hide all the controls + this.elements.controls.jelement.hide(); + else { + // else show the new control + this.elements.controls.jelement.show(); + this.elements.controlNew.jelement.show(); + } + + // only show the expand control in leaf nodes + if (node.isLeaf) + this.elements.controlExpand.jelement.show(); + else + this.elements.controlExpand.jelement.hide(); + }, // Events bindAllEvents: function() { // the artistmenu is always in sync with // the current playing track models.player.addEventListener('change', - this.onPlayerChange.bind(this)); + this.events.onPlayerChange.bind(this)); // the artistmenu always updates when a new node // as been selected this.graphcontroller.addGraphEvent('click', - this.onClickNode.bind(this)); + this.events.onClickNode.bind(this)); // Controls' Events this.elements.controlExpand.addDOMEvent({ eventName: 'onclick', - handler: this.onBtnExpandClick, + handler: this.events.onBtnExpandClick, context: this }); this.elements.controlNew.addDOMEvent({ eventName: 'onclick', - handler: this.onBtnNewClick, + handler: this.events.onBtnNewClick, context: this }); }, - onClickNode: function(data) { - var node = _.findWhere( - this.graphcontroller.artistGraph.data.nodes, { - id: parseInt(data.nodes[0]) - }); + events: { + // onclick a graph node event + onClickNode: function(data) { + var node = _.findWhere( + this.graphcontroller.artistGraph.data.nodes, { + id: parseInt(data.nodes[0]) + }); - if (!node || node.artist.uri === this.artist.uri) - return; + if (!node) + return; - if (node.id === 1) { - this.elements.controls.jelement.hide(); - } else { - this.elements.controlNew.jelement.show(); - this.elements.controls.jelement.show(); - } + this.updateView(node.artist); + }, + // on track change event + onPlayerChange: function() { + models.player.load('track').done(this, function(player) { - if (node.isLeaf) { - this.elements.controlExpand.jelement.show(); - } else { - this.elements.controlExpand.jelement.hide(); - } + var artist = models.Artist.fromURI( + models.player.track.artists[0].uri + ); - this.updateView(node.artist); - }, - onPlayerChange: function() { - models.player.load('track').done(this, function(player) { + // ignore if it's the same artist or an ad is playing + if ((this.artist && this.artist.uri === artist.uri) || + models.player.track.advertisement) { + return; + } - var artist = models.Artist.fromURI( - models.player.track.artists[0].uri + this.updateView(artist); + }); + }, + onBtnExpandClick: function(event) { + var node = _.findWhere( + this.graphcontroller.artistGraph.data.nodes, { + id: this.artist.nodeid + } ); - if ((this.artist && this.artist.uri === artist.uri) || - models.player.track.advertisement) { - return; - } - - this.updateView(artist); + node.color = { + border: '#7fb701', + background: '#313336' + }; + node.isLeaf = false; - this.elements.controls.jelement.show(); - this.elements.controlNew.jelement.show(); + this.graphcontroller.expandNode(this.artist); this.elements.controlExpand.jelement.hide(); - }); - }, - onBtnExpandClick: function(event) { - this.graphcontroller.showThrobber(); - - var node = _.findWhere( - this.graphcontroller.artistGraph.data.nodes, { - id: this.artist.nodeid - }); - - node.color = { - border: '#7fb701', - background: '#313336' - }; - node.isLeaf = false; - - this.artist.load('related').done(this, function(artist) { - var rootArtist = artist; - artist.related.snapshot(0, - this.graphcontroller.artistGraph.branching).done(this, - function(snapshot) { - snapshot.loadAll(['name', 'uri']).each(this, function(artist) { - var artistGraph = this.graphcontroller.artistGraph; - - var duplicated = _.findWhere(artistGraph.data.nodes, { - label: artist.name - }); - - if (duplicated && artist.name !== rootArtist.name) { - var inverseEdgeExists = _.findWhere(artistGraph.data.edges, { - from: duplicated.id, - to: rootArtist.nodeid - }); - var edgeExists = _.findWhere(artistGraph.data.edges, { - to: duplicated.id, - from: rootArtist.nodeid - }); - - if (!inverseEdgeExists && !edgeExists) { - var extraEdge = { - from: rootArtist.nodeid, - to: duplicated.id, - }; - - artistGraph.extraEdges.push(extraEdge); - - if (!artistGraph.treemode) - artistGraph.data.edges.push(extraEdge); - } - } else { - var nodeid = ++artistGraph.currentNodeId; - - artistGraph.data.nodes.push({ - id: nodeid, - label: artist.name, - artist: artist, - isLeaf: true - }); - - artistGraph.data.edges.push({ - from: rootArtist.nodeid, - to: nodeid - }); - - artistGraph.relatedArtists.push(artist); - - artist.nodeid = nodeid; - } - }).done(this, function() { - - this.graphcontroller.artistGraph.drawGraph(true); - }); - }); - }); - - this.elements.controlExpand.jelement.hide(); - }, - onBtnNewClick: function(event) { - this.graphcontroller.setArtistGraph(this.artist); - this.elements.controlNew.jelement.hide(); + }, + onBtnNewClick: function(event) { + this.graphcontroller.newGraph(this.artist); + this.elements.controlNew.jelement.hide(); + } } }); diff --git a/js/controllers/graphcontroller.js b/js/controllers/graphcontroller.js index 7af0d5f..35cf0cd 100644 --- a/js/controllers/graphcontroller.js +++ b/js/controllers/graphcontroller.js @@ -40,7 +40,7 @@ require([ // this.nowplayingArtist refers to the artist // of the current playing track this.nowplayingArtist = player.track.artists[0]; - this.setArtistGraph(this.nowplayingArtist); + this.newGraph(this.nowplayingArtist); }); }, @@ -64,7 +64,7 @@ require([ The parameter artist (if defined) is used as the root node. Otherwise, the this.nowplayingArtist is used. */ - setArtistGraph: function(artist) { + newGraph: function(artist) { var config = { options: this.options }; @@ -79,7 +79,7 @@ require([ this.artistGraph = new ArtistGraph( this.element, - artist || this.nowplayingArtist, + artist, config ); @@ -97,7 +97,13 @@ require([ updateData: function() { this.artistGraph.updateData(); }, - + expandNode: function(artist) { + this.artistGraph.constructGraph( + 0, + artist, + this.updateData.bind(this) + ); + }, // Displays a loading throbber and hides the graph canvas showThrobber: function() { if (this.artistGraph.throbber) diff --git a/js/models/artistgraph.js b/js/models/artistgraph.js index 1f87753..fdeb01e 100644 --- a/js/models/artistgraph.js +++ b/js/models/artistgraph.js @@ -54,6 +54,25 @@ ArtistGraph.DEFAULT_DEPTH = 2; ArtistGraph.DEFAULT_TREEMODE = true; ArtistGraph.DEFAULT_OPTIONS = {}; +// ArtistGraph.colors = { +// node: { +// border: "white", +// background: "black", +// highlight: { +// border: "white", +// background: "black" +// } +// }, +// edge: { +// border: "white", +// background: "black", +// highlight: { +// border: "white", +// background: "black" +// } +// } +// }; + ArtistGraph.prototype = { // initiates state properties of the graph @@ -85,7 +104,9 @@ ArtistGraph.prototype = { artist: this.artist, // isLeaf simply indicates if the node is a leaf // in the graph or not - isLeaf: false + isLeaf: false, + // is this the root node? + isRoot: true }], edges: [] }; @@ -100,7 +121,7 @@ ArtistGraph.prototype = { buildGraph: function() { // Current number of iterations (recursive calls) // done to construct the graph - this.currentIterations = 1; + var currentIteration = 1; // Maximum number of iterations that will be performed // to construct the graph. @@ -110,7 +131,7 @@ ArtistGraph.prototype = { (i < this.depth ? lambda.bind(this)(i + 1) : 0); }).bind(this)(0); - // this.maxIterations is equal to: + // maxIterations is equal to: // // d // ∑ b^i @@ -128,13 +149,32 @@ ArtistGraph.prototype = { // i = 0 // start constructing the graph recursively - this.constructGraph(this.depth - 1, this.artist); + this.constructGraph( + this.depth - 1, + this.artist, + iterationUpdate.bind(this) + ); + + function iterationUpdate() { + // Update the number of iterations done and + currentIteration += this.branching; + + // If the number of iterations done is enough to have the + // full graph constructed, then stop recursion and + // draw the final graph. + if (currentIteration === this.maxIterations) { + this.drawGraph(true); + } + } }, // Constructs the graph by recursively decreasing the depth // parameter and using the correct rootArtist on each - // recursive call - constructGraph: function(depth, rootArtist) { + // recursive call. + // The update parameter is a callback to be called + // after all the callbacks of the nodes of a node expansion + // have finished. + constructGraph: function(depth, rootArtist, update) { // load the related artists property rootArtist.load('related').done(this, function(artist) { @@ -145,8 +185,10 @@ ArtistGraph.prototype = { // when done loading, load name and uri properties // of each artist in the snapshot snapshot.loadAll(['name', 'uri']) - // when done, call forEachRelated on each artist - .each(this, forEachRelated); + // call forEachRelated on each artist + .each(this, forEachRelated) + // when done on each artist update the number of iterations + .done(update); }); }); @@ -234,15 +276,7 @@ ArtistGraph.prototype = { // constructing the graph, now with the current artist // as the rootArtist if (depth > 0) - this.constructGraph(depth - 1, artist); - - // Update the number of iterations done and - // If the number of iterations done is enough to have the - // full graph constructed, then stop recursion and - // draw the final graph. - if (++this.currentIterations === this.maxIterations) { - this.drawGraph(true); - } + this.constructGraph(depth - 1, artist, update); } }, From 8489b634dc4645eae5b88a303564dead2c5cff6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Z=C3=A9=20Bateira?= Date: Fri, 23 May 2014 16:36:26 +0100 Subject: [PATCH 3/5] artistmenu, graphcontroller and artistgraph ref --- Gruntfile.js | 2 +- js/controllers/artistmenu.js | 19 +---- js/controllers/graphcontroller.js | 118 +++++++++++++++++------------- js/controllers/tagsmenu.js | 4 +- js/models/artistgraph.js | 111 ++++++++++++++++------------ 5 files changed, 137 insertions(+), 117 deletions(-) diff --git a/Gruntfile.js b/Gruntfile.js index f533b90..6fcd514 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -29,7 +29,7 @@ module.exports = function(grunt) { }, watch: { files: ['Gruntfile.js', 'js/**/*.js', 'sass/*.scss', 'test/*.js'], - tasks: ['compass', 'jshint', 'jasmine'] + tasks: ['compass', 'jshint'] }, useminPrepare: { html: 'index.html', diff --git a/js/controllers/artistmenu.js b/js/controllers/artistmenu.js index 8bb118e..21eb818 100644 --- a/js/controllers/artistmenu.js +++ b/js/controllers/artistmenu.js @@ -225,7 +225,7 @@ require([ // based on the artist updateControls: function(artist) { var node = _.findWhere( - this.graphcontroller.artistGraph.data.nodes, { + this.graphcontroller.getData().nodes, { label: artist.name }); @@ -283,7 +283,7 @@ require([ // onclick a graph node event onClickNode: function(data) { var node = _.findWhere( - this.graphcontroller.artistGraph.data.nodes, { + this.graphcontroller.getData().nodes, { id: parseInt(data.nodes[0]) }); @@ -309,28 +309,17 @@ require([ this.updateView(artist); }); }, + // Expand control button click event onBtnExpandClick: function(event) { - var node = _.findWhere( - this.graphcontroller.artistGraph.data.nodes, { - id: this.artist.nodeid - } - ); - - node.color = { - border: '#7fb701', - background: '#313336' - }; - node.isLeaf = false; - this.graphcontroller.expandNode(this.artist); this.elements.controlExpand.jelement.hide(); }, + // New control button click event onBtnNewClick: function(event) { this.graphcontroller.newGraph(this.artist); this.elements.controlNew.jelement.hide(); } } - }); exports.artistmenu = ArtistMenu; diff --git a/js/controllers/graphcontroller.js b/js/controllers/graphcontroller.js index 35cf0cd..f267cf9 100644 --- a/js/controllers/graphcontroller.js +++ b/js/controllers/graphcontroller.js @@ -49,17 +49,17 @@ require([ // This is useful when the window is being resized and // the throbbers is not aligned at the center of the screen. updateView: function() { - if (this.artistGraph) { - this.artistGraph.redraw(); + if (this.artistgraph) { + this.artistgraph.redrawGraph(); - if (this.artistGraph.throbber) - this.artistGraph.throbber.setPosition('center', 'center'); + if (this.artistgraph.throbber) + this.artistgraph.throbber.setPosition('center', 'center'); } }, /** Set artist from the current playing track. - Creates this.artistGraph object. + Creates this.artistgraph object. The parameter artist (if defined) is used as the root node. Otherwise, the this.nowplayingArtist is used. @@ -71,34 +71,44 @@ require([ // if the artistGraph object as already been defined // then use the previous settings - if (this.artistGraph) { - config.branching = this.artistGraph.branching; - config.depth = this.artistGraph.depth; - config.treemode = this.artistGraph.treemode; + if (this.artistgraph) { + config.branching = this.artistgraph.branching; + config.depth = this.artistgraph.depth; + config.treemode = this.artistgraph.treemode; } - this.artistGraph = new ArtistGraph( + this.artistgraph = new ArtistGraph( this.element, artist, config ); this.showThrobber(); - this.artistGraph.buildGraph(); + this.artistgraph.buildGraph( + this.hideThrobber.bind(this) + ); this.bindAllEvents(); }, // Updates the graph given the new config values updateGraph: function(config) { this.showThrobber(); - this.artistGraph.updateGraph(config); + this.artistgraph.updateGraph(config, + this.hideThrobber.bind(this) + ); }, // Updates graph's data (nodes and edges) updateData: function() { - this.artistGraph.updateData(); + this.artistgraph.updateData(); }, + getData: function() { + return this.artistgraph.data; + }, + // Expands the specific given artist in the graph. + // the artist is required to be in the graph expandNode: function(artist) { - this.artistGraph.constructGraph( + this.artistgraph.highlightNode(artist); + this.artistgraph.expandNode( 0, artist, this.updateData.bind(this) @@ -106,14 +116,22 @@ require([ }, // Displays a loading throbber and hides the graph canvas showThrobber: function() { - if (this.artistGraph.throbber) - this.artistGraph.throbber.hide(); - - this.artistGraph.throbber = - Throbber.forElement(document.getElementById(this.name)); - this.artistGraph.throbber.setPosition('center', 'center'); - this.artistGraph.throbber._addBackground(); + if (this.throbber) { + this.throbber.hide(); + this.throbber.show(); + } else + this.throbber = + Throbber.forElement(document.getElementById(this.name)); + + this.throbber.setPosition('center', 'center'); + this.throbber._addBackground(); }, + hideThrobber: function() { + if (this.throbber) + this.throbber.hide(); + }, + + // Events // Binds all the events related to the graph components. bindAllEvents: function() { @@ -126,14 +144,35 @@ require([ models.player.load('track').done(onPlayerChange); }); - var artistgraph = this.artistGraph; - _.each(this.events, function(event) { - artistgraph.on(event.eventName, event.eventHandler); - }); + this.artistgraph.on(event.eventName, event.eventHandler); + }, this); _.each(this.graphevents, function(event) { - artistgraph.onGraph(event.eventName, event.eventHandler); + this.artistgraph.onGraph(event.eventName, event.eventHandler); + }, this); + }, + // Adds eventName to the ArtistGraph object given eventHandler, + // as a graph event. + addGraphEvent: function(eventName, eventHandler) { + this.artistgraph.onGraph(eventName, eventHandler); + + this.graphevents.push({ + eventName: eventName, + eventHandler: eventHandler + }); + }, + + // Adds eventName to the ArtistGraph object given eventHandler, + // as a custom event. The event should be run by ArtistGraph + // accordingly. + addCustomGraphEvent: function(eventName, eventHandler) { + if (this.artistgraph) + this.artistgraph.on(eventName, eventHandler); + + this.events.push({ + eventName: eventName, + eventHandler: eventHandler }); }, @@ -152,7 +191,7 @@ require([ onNodeDoubleClick: function(data) { // find the clicked node. - var node = _.findWhere(this.artistGraph.data.nodes, { + var node = _.findWhere(this.artistgraph.data.nodes, { id: parseInt(data.nodes[0]) }); @@ -164,31 +203,8 @@ require([ node.artist.load('compilations').done(function(artist) { models.player.playContext(artist.compilations); }); - }, - - // Adds eventName to the ArtistGraph object given eventHandler, - // as a graph event. - addGraphEvent: function(eventName, eventHandler) { - this.artistGraph.onGraph(eventName, eventHandler); - - this.graphevents.push({ - eventName: eventName, - eventHandler: eventHandler - }); - }, - - // Adds eventName to the ArtistGraph object given eventHandler, - // as a custom event. The event should be run by ArtistGraph - // accordingly. - addCustomGraphEvent: function(eventName, eventHandler) { - if (this.artistGraph) - this.artistGraph.on(eventName, eventHandler); - - this.events.push({ - eventName: eventName, - eventHandler: eventHandler - }); } + }); exports.graphcontroller = GraphController; diff --git a/js/controllers/tagsmenu.js b/js/controllers/tagsmenu.js index 6c92890..55d2a32 100644 --- a/js/controllers/tagsmenu.js +++ b/js/controllers/tagsmenu.js @@ -12,7 +12,7 @@ require([ } }); - TagsMenu.MAX_TAGS = 10; + TagsMenu.MAX_TAGS = 12; TagsMenu.implement({ loadController: function(graphcontroller) { @@ -26,7 +26,7 @@ require([ this.element.innerHTML = 'Loading tags...'; }, updateView: function() { - var nodes = this.graphcontroller.artistGraph.data.nodes; + var nodes = this.graphcontroller.artistgraph.data.nodes; this.resetView(); diff --git a/js/models/artistgraph.js b/js/models/artistgraph.js index fdeb01e..0b50dcc 100644 --- a/js/models/artistgraph.js +++ b/js/models/artistgraph.js @@ -118,7 +118,7 @@ ArtistGraph.prototype = { }, // Resets state variables and starts constructing the graph - buildGraph: function() { + buildGraph: function(done) { // Current number of iterations (recursive calls) // done to construct the graph var currentIteration = 1; @@ -149,7 +149,7 @@ ArtistGraph.prototype = { // i = 0 // start constructing the graph recursively - this.constructGraph( + this.expandNode( this.depth - 1, this.artist, iterationUpdate.bind(this) @@ -164,23 +164,24 @@ ArtistGraph.prototype = { // draw the final graph. if (currentIteration === this.maxIterations) { this.drawGraph(true); + if (done) + done(); } } }, - // Constructs the graph by recursively decreasing the depth - // parameter and using the correct rootArtist on each - // recursive call. - // The update parameter is a callback to be called - // after all the callbacks of the nodes of a node expansion + // Expands the node of the parent artist by this.branching. + // It recursively decreases the depth parameter. + // The update parameter is the callback to be called + // after all the callbacks of the child nodes of the root node // have finished. - constructGraph: function(depth, rootArtist, update) { + expandNode: function(depth, parentArtist, update) { // load the related artists property - rootArtist.load('related').done(this, function(artist) { + parentArtist.load('related').done(this, function(parentArtist) { // when done loading, load the current snapshot of the array // of artists, with this.branching length - artist.related + parentArtist.related .snapshot(0, this.branching).done(this, function(snapshot) { // when done loading, load name and uri properties // of each artist in the snapshot @@ -193,40 +194,45 @@ ArtistGraph.prototype = { }); // Updates the graph given the artist parameter. - function forEachRelated(artist) { + // This function will be called on each child node + // (artist parameter) of the root node. + // note: this means that this function will be called + // exactly this.branching times. + function forEachRelated(childArtist) { // Try to find repeated nodes in the graph // given the name of the artist is the same var duplicated = _.findWhere(this.data.nodes, { - label: artist.name + label: childArtist.name }); // Is the artist node already in the graph? // If there is a duplicate and if its not the same one, // then create and edge between the two artists: - // artist and rootArtist + // the child artist and parent artist // // The latter test was added after metadata errors were found: - // sometimes, an artist would exist itself in the related + // sometimes, an artist would exist itself in its related // artists list, which created a edge that went from it to // itself. - if (duplicated && artist.name !== rootArtist.name) { + if (duplicated && childArtist.name !== parentArtist.name) { // try to find repeated edges in the graph var edgeExists = _.findWhere(this.data.edges, { to: duplicated.id, - from: rootArtist.nodeid + from: parentArtist.nodeid }); - // find repeated edges (even if inverse) + // try to find repeated edges (even if inverse) var inverseEdgeExists = _.findWhere(this.data.edges, { from: duplicated.id, - to: rootArtist.nodeid + to: parentArtist.nodeid }); + // If no if (!edgeExists && !inverseEdgeExists) { // Create the extra edge. var extraEdge = { - from: rootArtist.nodeid, + from: parentArtist.nodeid, to: duplicated.id, }; this.extraEdges.push(extraEdge); @@ -253,8 +259,8 @@ ArtistGraph.prototype = { // then add it to the list of nodes this.data.nodes.push({ id: ++this.currentNodeId, - label: artist.name, - artist: artist, + label: childArtist.name, + artist: childArtist, // if the depth value of the graph is zero // then this is most definitely a leaf node isLeaf: depth <= 0 @@ -263,20 +269,21 @@ ArtistGraph.prototype = { // also create the edge to connect the new node to // its parent this.data.edges.push({ - from: rootArtist.nodeid, + from: parentArtist.nodeid, to: this.currentNodeId }); - this.relatedArtists.push(artist); + this.relatedArtists.push(childArtist); - artist.nodeid = this.currentNodeId; + childArtist.nodeid = this.currentNodeId; } // if a leaf node as not been reached, then continue - // constructing the graph, now with the current artist - // as the rootArtist + // constructing the graph, now with this child node + // as the root node if (depth > 0) - this.constructGraph(depth - 1, artist, update); + this.expandNode(depth - 1, childArtist, update); + // note: the condition to end the recursion is: if depth <= 0 } }, @@ -287,6 +294,7 @@ ArtistGraph.prototype = { this.bindAllGraphEvents(); // sets the previously computed graph data + // note: no animation starts at this point this.graph.setData(this.data, { disableStart: true }); @@ -298,14 +306,6 @@ ArtistGraph.prototype = { // the graph this.events.update(); - // The Spotify's views.Throbber object was initialized in the - // controllers.GraphController object to hide the graph canvas - // while the graph is being computed. - // Since that at this point the graph is ready to be shown, - // the throbber can now be hidden, and show the graph. - if (this.throbber) - this.throbber.hide(); - // Debug information about the graph creation if (debug) { console.log('#### Stats for ' + this.artist.name); @@ -315,10 +315,13 @@ ArtistGraph.prototype = { } }, + redrawGraph: function() { + this.graph.redraw(); + }, // Updates the graph with the given config object // it is expected that config is defined - updateGraph: function(config) { + updateGraph: function(config, done) { this.branching = config.branching || this.branching; this.depth = config.depth || this.depth; @@ -326,19 +329,39 @@ ArtistGraph.prototype = { this.treemode = config.treemode; this.reset(); - this.buildGraph(); + this.buildGraph(done); }, // Refresh vis.Graph's data object updateData: function() { this.graph.setData(this.data); }, - redraw: function() { - this.graph.redraw(); + highlightNode: function(artist) { + var node = _.findWhere( + this.data.nodes, { + id: artist.nodeid + } + ); + + // highlight the node + node.color = { + border: '#7fb701', + background: '#313336' + }; + // after expanding, the node will stop being a leaf + node.isLeaf = false; }, // Events + // binds the previously saved vis.Graph's events to the + // graph object + bindAllGraphEvents: function() { + for (var event in this.graphevents) { + this.graph.on(event, this.graphevents[event]); + } + }, + // saves the given event, given the proper eventHandler. on: function(event, eventHandler) { this.events[event] = eventHandler; @@ -347,15 +370,7 @@ ArtistGraph.prototype = { // saves a vis.Graph event, given the proper eventHandler onGraph: function(event, eventHandler) { this.graphevents[event] = eventHandler; - }, - - // binds the previously saved vis.Graph's events to the - // graph object - bindAllGraphEvents: function() { - for (var event in this.graphevents) { - this.graph.on(event, this.graphevents[event]); - } - }, + } }; ArtistGraph.prototype.constructor = ArtistGraph; From 0b800075529f4aa0d533f5f68c4992959fdfee87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Z=C3=A9=20Bateira?= Date: Fri, 23 May 2014 19:21:04 +0100 Subject: [PATCH 4/5] updateData now doesnt force graph rendering. OMG --- js/controllers/graphcontroller.js | 3 +++ js/controllers/tagsmenu.js | 7 +++++-- js/models/artistgraph.js | 10 ++++++++-- js/models/element.js | 2 +- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/js/controllers/graphcontroller.js b/js/controllers/graphcontroller.js index f267cf9..b9db0b6 100644 --- a/js/controllers/graphcontroller.js +++ b/js/controllers/graphcontroller.js @@ -101,6 +101,9 @@ require([ updateData: function() { this.artistgraph.updateData(); }, + updateNodes: function() { + this.artistgraph.updateNodes(); + }, getData: function() { return this.artistgraph.data; }, diff --git a/js/controllers/tagsmenu.js b/js/controllers/tagsmenu.js index 55d2a32..23a829e 100644 --- a/js/controllers/tagsmenu.js +++ b/js/controllers/tagsmenu.js @@ -2,6 +2,7 @@ require([ '$api/models', 'js/controllers/controller#controller' ], function(models, Controller) { + var TagsMenu = new Class({ Extends: Controller, @@ -12,9 +13,11 @@ require([ } }); + // Maximum number of tags shown TagsMenu.MAX_TAGS = 12; TagsMenu.implement({ + loadController: function(graphcontroller) { this.graphcontroller = graphcontroller; @@ -26,7 +29,7 @@ require([ this.element.innerHTML = 'Loading tags...'; }, updateView: function() { - var nodes = this.graphcontroller.artistgraph.data.nodes; + var nodes = this.graphcontroller.getData().nodes; this.resetView(); @@ -104,7 +107,7 @@ require([ }; }); - graphcontroller.updateData(); + graphcontroller.updateNodes(); _.each(this.nodes, function(node) { if (node.id !== 1) diff --git a/js/models/artistgraph.js b/js/models/artistgraph.js index 0b50dcc..7637684 100644 --- a/js/models/artistgraph.js +++ b/js/models/artistgraph.js @@ -332,9 +332,15 @@ ArtistGraph.prototype = { this.buildGraph(done); }, - // Refresh vis.Graph's data object + // Refresh vis.Graph's data objects updateData: function() { - this.graph.setData(this.data); + this.graph.nodesData.update(this.data.nodes); + this.graph.edgesData.update(this.data.edges); + + this.events.update(); + }, + updateNodes: function() { + this.graph.nodesData.update(this.data.nodes); }, highlightNode: function(artist) { var node = _.findWhere( diff --git a/js/models/element.js b/js/models/element.js index 6f91d07..17e7e75 100644 --- a/js/models/element.js +++ b/js/models/element.js @@ -8,7 +8,7 @@ var Element = function(selector) { this.element = this.jelement[0]; }; -// Instances only methods +// Instance only methods Element.prototype = { addDOMEvent: function(config) { this.element[config.eventName] = From 82f7c0ca499c4b5097e2c654d16a293faa65e436 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Z=C3=A9=20Bateira?= Date: Fri, 23 May 2014 19:32:19 +0100 Subject: [PATCH 5/5] v0.10.5 - fixes #32 --- README.md | 10 +++++++--- install.sh | 2 +- manifest.json | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 073bc7c..1277e91 100644 --- a/README.md +++ b/README.md @@ -31,9 +31,9 @@ These are the contents of the script: ```sh $ mkdir ~/Spotify ; cd ~/Spotify $ rm -rf rama -$ rm rama_v0.10.2.tar.gz -$ curl https://github.com/carsy/rama-spotify/releases/download/v0.10.2/rama_v0.10.2.tar.gz -$ tar -xvf rama_v0.10.2.tar.gz +$ rm rama_v0.10.5.tar.gz +$ curl https://github.com/carsy/rama-spotify/releases/download/v0.10.5/rama_v0.10.5.tar.gz +$ tar -xvf rama_v0.10.5.tar.gz $ open spotify:app:rama ``` @@ -60,6 +60,9 @@ If not, restart Spotify and then open the app by typing spotify:app:rama in the [Releases] ---- +[v0.10.5] - GraphController bug fixes + - when data is updated, the graph does not re-render + [v0.10.2] - Tags menu bug fixes - nodes inserted by expanding node were not being highlighted by tags @@ -125,6 +128,7 @@ José Bateira [here]:https://github.com/carsy/rama-spotify/releases/latest [Releases]:https://github.com/carsy/rama-spotify/releases/latest [issues]:https://github.com/carsy/rama-spotify/issues +[v0.10.5]:https://github.com/carsy/rama-spotify/releases/tag/v0.10.5 [v0.10.2]:https://github.com/carsy/rama-spotify/releases/tag/v0.10.2 [v0.10]:https://github.com/carsy/rama-spotify/releases/tag/v0.10 [v0.9.1]:https://github.com/carsy/rama-spotify/releases/tag/v0.9.1 diff --git a/install.sh b/install.sh index 4bd88b5..51b4870 100644 --- a/install.sh +++ b/install.sh @@ -1,6 +1,6 @@ #!/bin/bash -version="v0.10.2" +version="v0.10.5" mkdir ~/Spotify ; cd ~/Spotify rm -rf rama-spotify diff --git a/manifest.json b/manifest.json index 570f3d6..7d79821 100644 --- a/manifest.json +++ b/manifest.json @@ -11,7 +11,7 @@ "track" ], "BundleType": "Application", - "BundleVersion": "0.10.2", + "BundleVersion": "0.10.5", "Dependencies": { "api": "1.0.0", "views": "1.0.0"