Custom menu types "input" and "slider" values not included in notification payload.

[tonymin]

Bug Report

Description

Playing around with the new "slider" and "input" types, I couldn't figure out how to get the text and slider data from the notification payload. I looked through the source code, and it looks to me that the "value" property is not embedded into the payload.

Expected behavior

Payload from slider notification contains the slider value.

Current behavior

Payload from slider notification does not contains the slider value.

Possible solution

File: remote.js
Function: createMenuElement()

For the type "slider", replace the code:

this.sendSocketNotification("REMOTE_ACTION", Object.assign({ action: content.action.toUpperCase() }, { payload:{} }, content.content, { value: document.getElementById(`${content.id}-slider`).value }));

with:

this.sendSocketNotification("REMOTE_ACTION", Object.assign({ action: content.action.toUpperCase() }, content.content, { payload: Object.assign({}, content.content == undefined ? {} : content.content.payload, {value: document.getElementById(${content.id}-slider).value})}, { value: document.getElementById(${content.id}-slider).value }));

Similarly for the type "input", replace the code:

this.sendSocketNotification("REMOTE_ACTION", Object.assign({ action: content.action.toUpperCase() }, { payload:{} }, content.content, { value: document.getElementById(`${content.id}-input`).value }));

with:

this.sendSocketNotification("REMOTE_ACTION", Object.assign({ action: content.action.toUpperCase() }, content.content, { payload: Object.assign({}, content.content == undefined ? {} : content.content.payload, {value: document.getElementById(${content.id}-input).value})}, { value: document.getElementById(${content.id}-input).value }));

The proposed solution embeds the value of the slider/input inside the "payload" object.

[ezeholz]

I'll check it out when I have time. In the meantime, maybe you could try creating a PR with those changes you proposed. Thanks for letting me know that. Cheers :D

[ezeholz]

I just saw your proposal, seems fine. But it's actually needed the value inside of the payload? I believe that maybe could be redundant, because the value it's already included inside of the general notification. Instead of using payload.value from the notification, you can use value.
This change could also be a breaking change to people that use string values from the payload.
Maybe it's needed for some apps, and I'll totally understand it, but more than a bug, could be a future feature that some people could implement if they need it.

[tonymin]

• "Instead of using payload.value from the notification, you can use value."
  • I'm not sure if I understand you correctly, but for an external module to receive the value, it must be embedded into the notification payload right?

From my understanding, the code snippet below forwards the web app notifications to be broadcasted to external modules. However, if query.value is not embedded into payload, external modules won't actually receive the value.
File: node_helper.js
Function: executeQuery

 if (query.action === 'NOTIFICATION') {
    try {
        var payload = {}; // Assume empty JSON-object if no payload is provided
        if (typeof query.payload === 'undefined') {
            payload = query.payload;
        } else if (typeof query.payload === 'object') {
            payload = query.payload;
        } else if (typeof query.payload === 'string') {
            if (query.payload.startsWith("{")) {
                payload = JSON.parse(query.payload);
            } else {
                payload = query.payload;
            }
        }
        this.sendSocketNotification(query.action, { 'notification': query.notification, 'payload': payload });
        this.sendResponse(res);
        return true;
    } catch (err) {
        console.log("ERROR: ", err);
        this.sendResponse(res, err, { reason: err.message });
        return true;
    }
}

[ezeholz]

You're totally right. In a few days I'll be able to test it myself, and I'll add your commit inside the code. Thanks!