Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A couple of questions about ui_base.js #1068

Open
bartbutenaers opened this issue Jul 6, 2024 · 2 comments
Open

A couple of questions about ui_base.js #1068

bartbutenaers opened this issue Jul 6, 2024 · 2 comments
Labels
needs-triage Needs looking at to decide what to do

Comments

@bartbutenaers
Copy link
Contributor

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
Copy link
Contributor

colinl commented Jul 6, 2024

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.

1 similar comment
@colinl
Copy link
Contributor

colinl commented Jul 6, 2024

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.

@joepavitt joepavitt added the needs-triage Needs looking at to decide what to do label Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage Needs looking at to decide what to do
Projects
None yet
Development

No branches or pull requests

3 participants