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

Use editableList in form #1187

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

bartbutenaers
Copy link
Contributor

Description

The config screen of the ui-form node contains a custom build editable list, to mimic the Node-RED standard editableList. While it looks very nice, it makes it hard for developers (like me) to contribute. And it doesn't really help to get a consistent look and feel across the nodes.

Therefore I have replaced it by a standard editableList component:

form_editablelist

Feel free to tweek the css, to make it look and behave better! That isn't really my cup of tea.

BTW I did not find why there is a difference in the locales from the form and the node-red one's, so I handle those in two different ways:

image

Hopefully this gets merged soon, because I need it for another ui-form related pull request.

Thanks!

Related Issue(s)

#863

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

Hopefully this gets merged soon, because I need it for another ui-form related pull request.

You can always branch from your own branch, no need to pause development until this is merged

Copy link
Contributor

@gayanSandamal gayanSandamal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me

@gayanSandamal gayanSandamal self-requested a review August 14, 2024 10:01
@gayanSandamal
Copy link
Contributor

looks good to me

I'm rechecking this

Copy link
Contributor

@gayanSandamal gayanSandamal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR looks good to me.

It would be better if we could bring back the trash icon that was there before, but that can be addressed in a future update, not in this PR.

Screenshot 2024-08-14 at 16 25 43

@gayanSandamal gayanSandamal merged commit 511f310 into FlowFuse:main Aug 14, 2024
1 of 2 checks passed
@bartbutenaers
Copy link
Contributor Author

@gayanSandamal
Thanks for reviewing!!

It would be better if we could bring back the trash icon that was there before, but that can be addressed in a future update, not in this PR.

Well that was also my first thought. I was pretty sure that someone would give this feedback ;-)

Like I said above, I really like the look and feel of the original custom editableList. In fact - to be honest - my heart was a bit bleeding that I had to replace it by a standard editableList, because for me it looked like a little piece for nice CSS chart...

But imho it would be better if the standard editableList would be tweaked a bit, instead of just tweaking it for this ui node (or dashboard) only. A consistant look and feel across all (ui and non-ui) nodes would be nice.

@gayanSandamal
Copy link
Contributor

@gayanSandamal Thanks for reviewing!!

It would be better if we could bring back the trash icon that was there before, but that can be addressed in a future update, not in this PR.

Well that was also my first thought. I was pretty sure that someone would give this feedback ;-)

Like I said above, I really like the look and feel of the original custom editableList. In fact - to be honest - my heart was a bit bleeding that I had to replace it by a standard editableList, because for me it looked like a little piece for nice CSS chart...

But imho it would be better if the standard editableList would be tweaked a bit, instead of just tweaking it for this ui node (or dashboard) only. A consistant look and feel across all (ui and non-ui) nodes would be nice.

Thanks for the feedback @bartbutenaers , I agree, a consistent look and feel across all nodes would definitely enhance the UI. We’ll take this into account for future tweaks. Appreciate your insights! 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants