node-red-dashboard icon indicating copy to clipboard operation
node-red-dashboard copied to clipboard

Modify/Update UI-widget parameters from external

Open xX-Nexus-Xx opened this issue 1 year ago • 10 comments

Description

Hi,

in DB1 it was possible to modify/update UI-widget parameters via input (e.g msg.Range.min,msg.Range.max,msg,label for UI-Slider range).

Would it be possible to have the same flexibility with the new UI-widgets in DB2?

Below UI-Slider is just an example (and the one I need most) but the feature request wuld apply to all the UI-widgets with its parameters

image

thanks for considering

Have you provided an initial effort estimate for this issue?

I have provided an initial effort estimate

- [ ] https://github.com/FlowFuse/node-red-dashboard/issues/854
- [ ] https://github.com/FlowFuse/node-red-dashboard/issues/857
- [ ] https://github.com/FlowFuse/node-red-dashboard/issues/860
- [ ] https://github.com/FlowFuse/node-red-dashboard/issues/862
- [ ] https://github.com/FlowFuse/node-red-dashboard/issues/905
- [ ] https://github.com/FlowFuse/node-red-dashboard/issues/1000
- [ ] https://github.com/FlowFuse/node-red-dashboard/issues/1176
- [ ] https://github.com/FlowFuse/node-red-dashboard/issues/1177
- [ ] https://github.com/FlowFuse/node-red-dashboard/issues/1181
- [ ] https://github.com/FlowFuse/node-red-dashboard/issues/1180
- [ ] https://github.com/FlowFuse/node-red-dashboard/issues/1178
- [ ] https://github.com/FlowFuse/node-red-dashboard/issues/1179

xX-Nexus-Xx avatar May 07 '24 23:05 xX-Nexus-Xx

Hi @joepavitt, If you want I can have a look at this one.
Unless you need it fast, because that won't fit into my limited free time...

  1. Are there any best practices I need to take into account?
  2. I assume the dropdown node is again the best reference example for this?
  3. And I assume this is not limited to the slider node? If so, it might be better to create a PR per ui node?

bartbutenaers avatar May 08 '24 05:05 bartbutenaers

And I assume this is not limited to the slider node? If so, it might be better to create a PR per ui node?

We have a considerable backlog of dynamic properties that are needed - I may try and dedicate a day this week you just this.

I should be able to cover it, thanks for the offer @bartbutenaers

joepavitt avatar May 08 '24 07:05 joepavitt

Had already started work on dynamic props, with several PRs open following Pattern 1 from the below list. However, @Steve-Mcl has raised very valid concerns on this pattern, and he I are are currently talking through the options we have on architecture here:

Pattern 1 - <msg.property>

  • Any dynamic properties can be overridden with msg.<property>
  • This occurs even if a value was defined in the node configuration
    • Worth noting this is against the NR standard, which will only override, if a field is empty

Pros:

  • Easy to follow and understand
  • Easy for the user to define
  • Easy to implement from a development standpoint

Cons:

  • msg objects passed through nodes could then have a knock on effect, e.g. msg.label could end up running through many ui- nodes and override their label property
  • msg.<property> is ambiguous in that it could easily overlap accidentally with other, valid, uses of that same property
  • Invisible "magic" property, not in the configuration explicitly

Pattern 2 - <msg.ui_control.property>

  • Dashboard 1.0's method
  • Similar to pattern 1, but nests the overrides underneath a .ui (or equivalent) to minimise accidental overlap with valid uses of the existing msg properties

Pros:

  • Fixes 1 x on from Pattern 1

Cons:

  • Nested properties are likely to be more complex for low-code users to understand
  • Knock-on effect problem still persists
  • Invisible magic still persists

Pattern 3 - Typed Inputs

  • Utilise Node-RED's built-in "TypedInput", which permits a user to define where the property is defined (e.g. static string or dynamic msg property)

Pros:

  • Clear visibility on the definition of the property
  • Less likely to have knock-on effect to other nodes

Cons:

  • Developing with TypedInputs is not a fun DX
  • Can't set "default" value easily, if you want something driven by msg later, it also needs to be driven by msg at the start

Pattern 4 - Payload/Topic Pairing

  • msg.topic defines the property, msg.payload defines the value
  • Send this to a ui-control node, rather than the node itself

Pros:

  • No risk of knock-on effect

Cons:

  • User needs to find widget/node id in order to define the specific node in question which isn't low-code friendly
  • End up with a lot of wires into a single ui-control
  • More difficult to track back changes to a node given that the event updating it wires to a completely different node

joepavitt avatar May 16 '24 11:05 joepavitt

I am swaying towards Pattern 3 - but I'm not sure I have the energy to implement 50+ TypedInputs

joepavitt avatar May 16 '24 11:05 joepavitt

Given not all properties are suitable for a TypedInput style widget, I'd suggest starting with Pattern 2 - as it will be familiar to existing Dashboard users. It won't preclude Pattern 3 in the future if that's deemed a better solution later.

The knock-on effect could be mitigated by having a standard of clearing that property before passing the message on - they are single-use properties.

knolleary avatar May 16 '24 12:05 knolleary

The knock-on effect could be mitigated by having a standard of clearing that property before passing the message on - they are single-use properties.

I was in favour of this also

joepavitt avatar May 16 '24 12:05 joepavitt

Obviously I'm biased - but yes... the reasons we (I) went for pattern 2 (msg.ui_control.***) in dashboard 1 was precisely so it was obvious that it was to do with ui and not just picking up generic properties (like msg.label).

And also ... as you pointed out it is an anti pattern to the default Node-RED behaviour where if the author sets a config value then that is honoured first - and can't be overridden. So by using a sub-property I could justify to myself that this was safe enough to avoid users doing it accidentally.

The reason I wanted to allow this override "all over the UI" was that for the config I thought that in general you would want to be able to set a default value using the config field - but be able to override it later if necessary - rather then having to leave something blank and then having to send a valid property value with every message - which could make every single data point into a much larger object. IE not have to send min, max, label etc with every point...

(The technical debt of this is - as someone else has pointed out recently - that in dashboard 1 we only ever retained the payload value over a refresh - so labels, max, min etc would all reset to default on a refresh (or even on a navigate away to another tab and back) - which meant they have to resend those extra control properties when they spot the fresh happening... which is currently less than ideal)

dceejay avatar May 16 '24 16:05 dceejay

(The technical debt of this is - as someone else has pointed out recently - that in dashboard 1 we only ever retained the payload value over a refresh - so labels, max, min etc would all reset to default on a refresh (or even on a navigate away to another tab and back) - which meant they have to resend those extra control properties when they spot the fresh happening... which is currently less than ideal)

For what it's worth, I do already have a workaround for this as I have a server-side state store which retains the dynamic values, and populates them on a client refresh, etc.

It does however get lost on a Node-RED restart

joepavitt avatar May 16 '24 16:05 joepavitt

Probably fair enough to lose it on a restart - as the rest of the back end may have missed other inputs, and not be in a real/valid state at that point. If you can retain things over a browser refresh then you are already in a better place than dashboard 1.

dceejay avatar May 16 '24 19:05 dceejay

Taking into account the above, going to move forward with a msg.ui_update.<property> approach. Chosen update instead of control to differentiate from the control node, which this doesn't use at all.

The next question is whether I apply this at core such that it auto-applies to every node and for every property (which some exceptions like id, width, height that I'll throw in). The disadvantage of this is that it will also automatically apply to all third-party nodes, and possibly for properties we don't want to be dynamic. The other problem is that there is still front-end work required to handle the properties, and so, by auto-implementing the server-side, but no the front-end, it will likely result in some broken dashboards.

My thinking is, stick with msg.ui_update, and then move forward with the existing approaches/PRs that update this on a widget-by-widget and property-by-property basis

joepavitt avatar May 23 '24 14:05 joepavitt