FlowFuse / node-red-dashboard

https://dashboard.flowfuse.com
Apache License 2.0
181 stars 45 forks source link

A couple of questions about ui_base.js #1068

Open bartbutenaers opened 2 months ago

bartbutenaers commented 2 months ago

Hi guys,

I was browsing through the ui_base.js code, in order to gain some understanding to fill my knowledge gaps.
But there are a couple of things that are not clear to me:

  1. The defaultHandler for the onSend event has a "value" input parameter is not used. I did not have a look at what this code is supposed to do by default, but it looks like a bug to me.

  2. When there is no passthrough property, in the else statement the message is being send anyway. I might be mistaken, but that looks weird to me. Would be nice if a comment could be added to the code, to explain why this is required.

  3. In the code a few times a defaultHandler is being used, in case the widget itself does not provide a custom handler. But only for the onInput event there is an else statement, so imho it would be more clear for contributors if this else statement code snippet would be wrapped into a defaultHandler function.

Thanks!! Bart

colinl commented 2 months ago

Thanks for posting the links to that code. I see in there

                        // standard dynamic property handlers
                        if (hasProperty(msg, 'enabled')) {
                            statestore.set(n, widgetNode, msg, 'enabled', msg.enabled)
                        }
                        if (hasProperty(msg, 'visible')) {
                            statestore.set(n, widgetNode, msg, 'visible', msg.visible)
                        }
                        if (hasProperty(msg, 'class') || (hasProperty(msg, 'ui_update') && hasProperty(msg.ui_update, 'class'))) {
                            const cls = msg.class || msg.ui_update?.class
                            statestore.set(n, widgetNode, msg, 'class', cls)
                        }

so the standard message handler automatically updates enabled, visible and class in the statestore if they are present in the msg. So in third party nodes it is not necessary to maintain those oneself, server side.

colinl commented 2 months ago

Thanks for posting the links to that code. I see in there

                        // standard dynamic property handlers
                        if (hasProperty(msg, 'enabled')) {
                            statestore.set(n, widgetNode, msg, 'enabled', msg.enabled)
                        }
                        if (hasProperty(msg, 'visible')) {
                            statestore.set(n, widgetNode, msg, 'visible', msg.visible)
                        }
                        if (hasProperty(msg, 'class') || (hasProperty(msg, 'ui_update') && hasProperty(msg.ui_update, 'class'))) {
                            const cls = msg.class || msg.ui_update?.class
                            statestore.set(n, widgetNode, msg, 'class', cls)
                        }

so the standard message handler automatically updates enabled, visible and class in the statestore if they are present in the msg. So in third party nodes it is not necessary to maintain those oneself, server side.