nuvola-player-devel team mailing list archive

[Jiří Janoušek <janousek.jiri@xxxxxxxxx>]

Review: Needs Fixing

> I'm directly assigning this.naviagteBack.disabled again. The reason for not using the makeElement shorthand is, that this uses dom attributes and this means it will always be disabled (according to html spec, <button disable="anything"/> will disable the button).

It is possible to use elm.removeAttribute(), but you can keep it your way.

> I'm exposing Nuvola.ACTION_CHANGED instead of using "action-changed"

In that case you have to define it for older versions.

integration.js:
/* Monkey patching */
if (Nuvola.ACTION_CHANGED === undefined) // @since JS API 2.3
    Nuvola.ACTION_CHANGED = "action-changed";


> The feature is disabled on pre 2.4 versions.

In that case it would be better to move as much code as possible to shared file /data/nuvolaplayer/js/main.js. What do you think about it?

main.js
-------

// FIXME: documentation; backClass, backStyles, forwardClass, forwardStyles optional
Nuvola.NavigationButtons = function(backClass, backStyles, forwardClass, forwardStyles)
{
    this.backAction = Nuvola.Action(Nuvola.NavigationButtons.BACK);
    this.forwardAction = Nuvola.Action(Nuvola.NavigationButtons.FORWARD);
    this.backButton = Nuvola.makeElement("button", null, "<");
    this.forwardButton = Nuvola.makeElement("button", null, ">");
    if (backClass)
        this.backButton.className = backClass;
    if (backStyles)
        Nuvola.css(this.backButton, backStyles);
    if (backClass)
        this.forwardButton.className = forwardClass;
    if (forwardStyles)
        Nuvola.css(this.forwardButton, forwardStyles);
}

Nuvola.NavigationButtons.BACK = "back";
Nuvola.NavigationButtons.FORWARD = "forward";

// FIXME: documentation
Nuvola.NavigationButtons.prototype.update = function(name)
{
    var button = this[name + "Button"];
    var action = this[name + "Action"];
    if (button && action)
        button.disabled = !action.sensitive;
}

// FIXME: documentation
Nuvola.NavigationButtons.prototype.prepend = function(parent)
{
    if (!parent)
        return false;
    return this.insert(parent, parent.firstChild)
}

// FIXME: documentation
Nuvola.NavigationButtons.prototype.insert = function(parent, nextChild)
{
    
    if (!parent || !nextChild || this.parent)
        return false;
    
    var firstChild = parent.firstChild;
    parent.insertBefore(this.forwardButton, nextChild);
    parent.insertBefore(this.backButton, nextChild);
    this.parent = parent;
    return true; 
}

// FIXME: documentation
Nuvola.NavigationButtons.prototype.append = function(parent)
{
    if (!parent || this.parent)
        return false;
    
    var firstChild = parent.firstChild;
    parent.appendChild(this.forwardButton);
    parent.appendChild(this.backButton);
    this.parent = parent;
    return true; 
}

// FIXME: documentation
Nuvola.NavigationButtons.prototype.remove = function()
{
    if (!this.parent)
        return false;
    
    this.parent.removeChild(this.forwardButton);
    this.parent.removeChild(this.backButton);
    this.parent = undefined;
    return true;
}

// FIXME: documentation
Nuvola.css = function(elm, styles)
{  
    for (var style in styles)
        elm.style[style] = styles[style];
}

integration.js
--------------

/* Monkey patching */
if (Nuvola.ACTION_CHANGED === undefined) // @since JS API 2.3
    Nuvola.ACTION_CHANGED = "action-changed";

...

if (Nuvola.NavigationButtons !== undefined)
{
    this.navigationButtons = new Nuvola.NavigationButtons(...);
    this.navigationButtons.prepend(queryBar);
    ...
    this.navigationButtons.remove();
}

...

case Nuvola.ACTION_CHANGED: // @since JS API 2.3
if ((param1 == Nuvola.NavigationButtons.BACK || param1 == Nuvola.NavigationButtons.FORWARD) && param2 = "sensitive")
    navigationButtons.update(param1);

=== modified file 'data/nuvolaplayer/services/googleplay/settings.js'
152	+ Nuvola.config.navigationButtons === true,

Change to Nuvola.config.navigationButtons !== false. This is an useful feature and should be enabled by default (when Nuvola.config.navigationButtons === undefined !== true). (I might enable other extra features too.)
-- 
https://code.launchpad.net/~mpdeimos/nuvola-player/bug-1212167/+merge/200249
Your team Nuvola Player Development is subscribed to branch lp:nuvola-player.