Re-orgainse value/color table, fix spacing. Add library example.

Adorkable / node-red-contrib-ui-led

A simple LED status indicator for the Node-RED Dashboard

MIT License

16 stars 12 forks source link

Re-orgainse value/color table, fix spacing. Add library example. #43

Closed dceejay closed 4 years ago

dceejay commented 4 years ago

Hi, I have had a go at tidying up the spacing of the value / colour table slightly and swapped the order of the columns as I think it is more obvious to map value -> colour. I added the word msg.payload so that it is more obvious that is the value we are comparing.

I also added the emitOnlyNewValues: false, which should fix Issue #36.

And added an example to the examples dir so that it appears in the Editor - Import - Examples menu. Hopefully these are ok.

yoiang commented 4 years ago

Hey @dceejay , this is great, thanks so much for the PR!

Would it be alright if you or I split this into a few PRs, I'm thinking:

emitOnlyNewValues change - primarily because I was waiting on pushing it to see if there was a structural design that would negate that need but it's better to resolve #36 asap (I should, sorry y'all) and later revert emitOnlyNewValues
layout of editor - it looks great, I want to do a one to one comparison and just make sure I see the difference
examples directory - ah that's great to know that exists, I would want to convert all old examples over :)
node-red-dashboard as a peerDependency - can you elaborate on why it isn't a direct dependency?

How does that sound? I'd be happy to do the legwork, I appreciate your submission 😊

dceejay commented 4 years ago

emitOnlyNewValues is part of the API to try and reduce traffic from backend to the browser for unnecessary changes... - but in this instance you want the change to go through. So either you leave it broken, or allow it to resend the same data when it reconnects.
layout - I made the left gutter width slightly wider so the labels fitted onto one line and then swapped the order of the two value - color columns - and renamed it to be msg.payload to make it obvious that was what it was matching against... (until you change it to allow matching aginst other properties :-) (also slightly re-aligned the headings to sit over the columns)
examples - yes - that way they can be imported easily. Just ensure you only use core nodes and that they ideally work without any backend service (or credentials) - so they "just work".
if it's a full dependency it may load up the whole dashboard under your node_modules unless it matches the one that is overall installed (ie if you specify one version and the user has another installed) - if it's a peer then it will just warn that it needs to be installed but won't load. OK so it doesn't force load the dashboard if you don't have it - but having two would be really bad.

yoiang commented 4 years ago

Merged in dbe9c434c85578058d58bbcf08c588e4d0a11689, thanks again! It'll go out in the next release soon.

dceejay commented 4 years ago

excellent - thanks

yoiang commented 4 years ago

Thank you for the PR!