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

Standardise the property label instead title #1310

Conversation

gayanSandamal
Copy link
Contributor

@gayanSandamal gayanSandamal commented Sep 19, 2024

Description

@joepavitt @Steve-Mcl
As this related issue is only mentioned about the docs update but as per my understanding it should update all the areas of gauge node related with title but this will lead to a problem with the existing gauges that currently use title property in NR flows.

Screenshot 2024-09-19 at 15 37 52 Screenshot 2024-09-19 at 15 47 07

Therefore the dashboard users have to update their flows otherwise I should limit this PR just for a one line docs update

Screenshot 2024-09-19 at 15 47 31 Screenshot 2024-09-19 at 15 47 35

Related Issue(s)

#1284

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

The only request was to support ui_update.label in the dynamic properties functionality, and for thag to then, under the covers, update the title property.

The title field doesn't need to change, this should require no changes from the user.

@joepavitt
Copy link
Collaborator

The other option here is that we keep these changes, do move forward with label, but support injection of config.label if a config.title is present in the js file when the node is registered. This will prevent the need for users to manually update.

@Steve-Mcl can talk you through that process if required.

@Steve-Mcl
Copy link
Contributor

The title field doesn't need to change, this should require no changes from the user.

This is the preferred direction IMO. It will be seamless to the user. We will continue to use title in the flows and continue to support ui_update.title (but depreciate it) and support ui_update.label going forwards. Under the hood, ui_update.title and ui_update.label should be stored in the datastore as the same prop (I propose that prop is label). I also propose we do not mention title in the docs (even though it will continue to work as before)

Gayan and I have discussed this. If I get time today i will make the changes to this branch/PR otherwise, Gayan is to pick it up in the morning.

@gayanSandamal let me know is anything needs further clarification please. Ta.

@gayanSandamal
Copy link
Contributor Author

The title field doesn't need to change, this should require no changes from the user.

This is the preferred direction IMO. It will be seamless to the user. We will continue to use title in the flows and continue to support ui_update.title (but depreciate it) and support ui_update.label going forwards. Under the hood, ui_update.title and ui_update.label should be stored in the datastore as the same prop (I propose that prop is label). I also propose we do not mention title in the docs (even though it will continue to work as before)

Gayan and I have discussed this. If I get time today i will make the changes to this branch/PR otherwise, Gayan is to pick it up in the morning.

@gayanSandamal let me know is anything needs further clarification please. Ta.

Thank you so much Steve. I will talk to you if I need further clarifications on this

@Steve-Mcl
Copy link
Contributor

Approved. Merged only after tests pass

@Steve-Mcl
Copy link
Contributor

looks good - only the usual ✖ widgets/control.spec.js fail

@Steve-Mcl Steve-Mcl merged commit 312bbd4 into main Sep 20, 2024
1 of 2 checks passed
@Steve-Mcl Steve-Mcl deleted the 1284-inconsistent-use-of-msgui_updatetitle-to-update-the-label-for-gauge-node-msgui_updatelabel-is-used-for-other-nodes branch September 20, 2024 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants