ui-switch: sync issue

[bartbutenaers]

Description

When a ui-switch is displayed in two dashboards, their state seems not to be synced anymore. I assume that bug has been introduced by PR 1463.

It nows seems to be fixed by this PR:

The problem was caused because the ui-switch node had overridden the default onChange handler, but that handler was not refactored to push the new widget-sync event to the frontend. So the onSync function in the ui-switch Vue file was never called.

It was a bit more work as expected, because the default handler function (in ui-base) did not have a conn or id parameter. Instead it used - via closure - those parameters from the parent function. In order to have those parameters available in the ui-switch overridden onChange function, I had to add them as parameters. To make sure there are no breaking changes for third-party ui nodes, I have added the two new parameters at the end of the parameter list.

To be honest, I don't have a very good feeling with this one. The default handler functions are becoming more complex, and code needs to be duplicated every time. If you forget to keep the duplicated code in sync with all changes, you end up with issues like this one. Moreover existing third-party ui nodes won't use this new approach automatically. And if the third-party developer manually refactors his code to use the new mechanism, he will get breaking changes when his ui node is being used on an older dashboard version.

So it would be really useful if all the sync-related mechanisms could be finalized in the near future, to reduce the number of fixes and to avoid too much (repetetive) refactoring work. Imho that is quite high priority to get things stable... Thanks!!!

Related Issue(s)

1569: solves only one of the two problems in that ticket

Checklist

• I have read the contribution guidelines
• Suitable unit/system level tests have been added and they pass
• Documentation has been updated
  • Upgrade instructions
  • Configuration details
  • Concepts
• Changes flowforge.yml?
  • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
  • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

• Includes a DB migration? -> add the area:migration label

[joepavitt]

Great stuff as always @bartbutenaers

[colinl]

This (I assume it is this) appears to have broken the switch when passthrough is not enabled and Show State of Input is selected.
The switch changes state when it is clicked, whereas it should wait for an input.
This is on released version 1.21.0
Test flow

[{"id":"2a0f1d9a227c95e6","type":"inject","z":"997da33a0beedade","name":"","props":[{"p":"payload"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payload":"{\"state\": \"On\"}","payloadType":"json","x":190,"y":460,"wires":[["edb0c887d300f6cc"]]},{"id":"edb0c887d300f6cc","type":"ui-switch","z":"997da33a0beedade","name":"Test switch object values","label":"Test switch object","group":"7545fdb22e4a388c","order":6,"width":0,"height":0,"passthru":false,"decouple":false,"topic":"some/topic","topicType":"str","style":"","className":"","onvalue":"{\"state\": \"On\"}","onvalueType":"json","onicon":"","oncolor":"","offvalue":"{\"state\": \"Off\"}","offvalueType":"json","officon":"","offcolor":"","x":430,"y":500,"wires":[["b9d759a00e77582f"]]},{"id":"773ebd075c7df8f4","type":"inject","z":"997da33a0beedade","name":"","props":[{"p":"payload"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payload":"{\"state\": \"Off\"}","payloadType":"json","x":190,"y":520,"wires":[["edb0c887d300f6cc"]]},{"id":"b9d759a00e77582f","type":"debug","z":"997da33a0beedade","name":"debug 20","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"true","targetType":"full","statusVal":"","statusType":"auto","x":630,"y":500,"wires":[]},{"id":"7545fdb22e4a388c","type":"ui-group","name":"D2 tests","page":"d7fcdd33578e5d34","width":"6","height":"1","order":1,"showTitle":true,"className":"","visible":"true","disabled":"false"},{"id":"d7fcdd33578e5d34","type":"ui-page","name":"d2 P1","ui":"ID-BASE-1","path":"/d2p1","icon":"","layout":"flex","theme":"4bff2b59c4c518e1","order":7,"className":"","visible":"true","disabled":"false"},{"id":"ID-BASE-1","type":"ui-base","name":"Dashboard","path":"/dashboard","includeClientData":true,"acceptsClientConfig":["ui-control","ui-notification","ui-gauge-classic"],"showPageTitle":true,"titleBarStyle":"default"},{"id":"4bff2b59c4c518e1","type":"ui-theme","name":"Dark Theme","colors":{"surface":"#ffffff","primary":"#0094ce","bgPage":"#eeeeee","groupBg":"#231b1b","groupOutline":"#cccccc"},"sizes":{"pagePadding":"2px","groupGap":"2px","groupBorderRadius":"4px","widgetGap":"2px"}}]

[bartbutenaers]

@colinl
That is a pitty. It is quite difficult to get everything right without breaking stuff at the moment. If some community members could contribute a series of extra unit test flows, that would really be usefull (to get things tested automatically). Otherwise issues like yours unfortunately will keep popping up. Don't have time to contribute such stuff...

I will try to find some time tonight to have a look at your flow.

[colinl]

I keep meaning to look at how the unit tests work with a view to contributing. I am (metaphorically) snowed under at the moment though, but I will bump it up my priority list.

[bartbutenaers]

This (I assume it is this) appears to have broken the switch when passthrough is not enabled and Show State of Input is selected.

@colinl,
In your example flow you use the other option:

I have installed the release from yesterday (1.22) but at first sight I cannot see an issue. Could you explain a bit in more detail what is going wrong? Thanks!!

[bartbutenaers]

The switch changes state when it is clicked, whereas it should wait for an input.

I have the latest version installed:
11 Jan 18:56:09 - [info] [ui-base:My Dashboard] Node-RED Dashboard 2.0 (v1.22.0) started at /dashboard

But it "looks" normal to me:

I am going to put this aside, and wait for instructions how I can reproduce it.

[colinl]

Well, this is embarrassing. I could have sworn the flow I posted had Show State of Input, but I was wrong. I am really sorry to have wasted your time Bart. Now my brain is working again I see that it is all working exactly as expected.

[bartbutenaers]

Completely no problem @colinl!!
Thanks for confirming this.