nuvola-player-devel team mailing list archive

[mpdeimos <mpdeimos@xxxxxxxxx>]

Review: Resubmit

Resubmited. Some thoughts:

I know about removeElement, but this requires to 1) create the attributes object conditionally 2) use addElement/removeElement on update. This way it is much simpler.


The whole navigation bar integration is now a separate object in main.js as you suggested. This keeps code much simpler and reusable across services (I might also have a look at amazon and bandcamp integration). In the end I decided against taking your proposal as-is:
 * Creating the element will be handled by the service integration. There are lots of corner cases where you might wish to add some extra stuff. So just pass the back/forward button elements to the constructor.
 * I did not like feeding the integration object with message handler callbacks. Hence I've completely rewritten message handling for JS API 2.3 onwards. main.js defines a default message handler Nuvola.onMessageReceived that dispatches messages to handlers registered with Nuvola.onMessageReceived.addHandler(). Now the service integration as well the navigation button can register separate handlers. Pre JS API 2.3 will just override Nuvola.onMessageReceived as before, so this shouldn't be a problem (and navigation buttons are not available there anyways). I alo had to disable throwing exceptions in the message handler of the service integration, but using logging instead. Should yield the same result anyways.
 * The integration can be disabled either by calling deconstruct or setEnabled(false). The latter will just manipulate the element style to be hidden (used for google).
* Integration for navigation buttons is enabled by default.

Minor:
* Nuvola.Action and Nuvola.NavigationMuttonIntegration are wrapped in additional curly brackets to indicate that they are separate objects and support readability.
* I'm not sure if my integration code somehow messes with hide googleplus controls (searchbar is not stretched 100%). Hence I've adjusted the CSS there. Related: I'm not sure if the code below /* Fix bar dimensions */ is actually doing something and whether it can be removed.

-- 
https://code.launchpad.net/~mpdeimos/nuvola-player/bug-1212167/+merge/200249
Your team Nuvola Player Development is subscribed to branch lp:nuvola-player.