bartbutenaers / node-red-contrib-ui-svg

A Node-RED widget node to show interactive SVG (vector graphics) in the dashboard
Apache License 2.0
94 stars 27 forks source link

RFE: indefinite checkbox for repeatCount #94

Closed hobbes1069 closed 3 years ago

hobbes1069 commented 3 years ago

SVG allows for the repeatCount to be "indefinite", in the context of NodeRED, I would think a checkbox that greys out the numerical box would be the best way to handle it.

I've got a "Fan" SVG that I'm animating when the temperature goes over a certain value and I'd like it to stay running until the temperature drops below the set point. I could use a crazy high value but the spec allows for "indefinite".

bartbutenaers commented 3 years ago

Hi Richard (@hobbes1069),

That indeed makes sense. Unfortunately I'm not developing at the moment, due to a surgery. So you will have to wait a bit ... But I have added it to my (growing) todo list!

Kind regards, Bart

hobbes1069 commented 3 years ago

Well I'm not enough of a JS programmer to do it myself but I might try to get some pointers from the forums. If successful I'll submit a PR.

Steve-Mcl commented 3 years ago

Hi.

Can I suggest instead of an extra checkbox, if 0 is an invalid value for the animation count that we simply allow 0 & interpret it as infinity?

UI mods are a pain.

hobbes1069 commented 3 years ago

Actually it looks like that's already the case:

if (smilAnimation.repeatCount === "0") {
          animationElement.setAttribute("repeatCount"  , "indefinite");
}

It's just not in the documentation AFAIKT. Admittedly the documentation is quite long compared to other node types.

Steve-Mcl commented 3 years ago

I thought it was. I might have even written that line 😂

bartbutenaers commented 3 years ago

It's just not in the documentation AFAIKT.

Thanks guys for sorting this out! I will add it to the readme later on...

Admittedly the documentation is quite long compared to other node types.

And on your request it now will get longer again :-)

bartbutenaers commented 3 years ago

Seems it was already mentioned in the documentation, so I didn't need to add anything ...

image