FlowFuse / node-red-dashboard

https://dashboard.flowfuse.com
Apache License 2.0
207 stars 51 forks source link

Text Input node set for 'datetime picker' with a value passed to the input node breaks the picker #1409

Open gfinleyg opened 1 month ago

gfinleyg commented 1 month ago

Current Behavior

Inputs to a Text Input node that is set to datetime picker aren't displayed.

Expected Behavior

Passing a valid JavaScript Date number should change the date and time set in the field.

Steps To Reproduce

Attach a function block outputting msg.payload = 1728932496400 to the input of a Text Input node with it's mode set to datetime picker.

Environment

Have you provided an initial effort estimate for this issue?

I am not a FlowFuse team member

gfinleyg commented 1 month ago

The node's behavior is broken with this setup too, if you try to change any of the date/time fields or select using the calendar, it goes back to a blank value as soon as you mouse-hover over the field again.

bartbutenaers commented 1 month ago

@gfinleyg

gfinleyg commented 1 month ago

I was trying it with a numerical value of 1728932496400, it's good to note that the string "1728932496400" doesn't work either.

I also tried a few of the Javascript "new Date()" formats, with no luck.

It looks like using the format that you saw in the dev tools console log works and doesn't leave the tool broken in Firefox. It appears that the .toISOString() function would get that output, but none of the other formats will work out of the box and .toISOString() isn't available in my instance of Node-Red for function nodes.

colinl commented 1 month ago

I think the idea is that you have to feed it with a value in the same format as the output. If you want it to take a js timestamp then it has to know which timezone you want, which can be problematic. As it is, it is timezone agnostic. D1 is a bit of a mess with time/date pickers because of this. Edit: A mess because it tries to allow for timezone.

gfinleyg commented 1 month ago

@colinl That makes sense, but it isn't returned in that format and there isn't any documentation of how it should work in https://dashboard.flowfuse.com/nodes/widgets/ui-text-input.html. A working datetime output looks like: 12/01/2024, 01:01 AM not "yyyy-MM-ddThh:mm"

I'm not sure where to get access to the developer tools console log mentioned earlier, so I had no idea that was the format required for this to work.

I figured the varying format possibilities could be a problem which was why I started with the numerical ms since the beginning of the Epoch input.

colinl commented 1 month ago

but it isn't returned in that format

It is for me. image

gfinleyg commented 1 month ago

Fair, I was looking at the output on the dashboard rather than a debug node. I suppose not using the ms since the beginning of the epoch also allows usage of dates before 1970 which could have some use-cases.

Looks like you can use negative numbers to set dates prior to 1970, so that last statement isn't as important.

Steve-Mcl commented 2 weeks ago

Inputs to a Text Input node that is set to datetime picker aren't displayed.

They are if you send a well formatted date / time / datetime

See the "Try Demo" link in the docs: https://dashboard.flowfuse.com/nodes/widgets/ui-text-input.html

Related: https://discourse.nodered.org/t/default-values-in-text-input/93073


@joepavitt we may wish to relax this as it took me too long to determine the correct format. We should take anything that "looks like a date/time/datetime" and use it. As it stands, sending 2024-11-11 10:15 into a date is rejected, sending 2024-11-11 into a datetime is rejected - I see no reason to be so strict here?. We should also accept valid Epoch and perhaps Unix timestamp too. However, I do expect complications where timezones are concerned.

colinl commented 2 weeks ago

Be very careful here, I suggest not allowing anything where timezones may come into play, otherwise we will end up in the same mess as with D1. Currently the input has to be in the same form as the output, which is TZ agnostic and there is no confusion about what it means. That means, I think, that milliseconds, date/time with timezone specified, etc should not be allowed.

gfinleyg commented 2 weeks ago

@colinl Using milliseconds is setting it based on unix time which is always tied to UTC. There's no time zone to worry about with it.

colinl commented 2 weeks ago

Using milliseconds is setting it based on unix time which is always tied to UTC. There's no time zone to worry about

Suppose I have an Inject node configured to send the current time in milliseconds, feeding into the datetime picker node. If I look at my watch and it says 12:00 and I click the Inject node, what time would you expect it to set the picker to?

gfinleyg commented 2 weeks ago

@colinl That would be whatever difference you have between your Time Zone at 12:00 and UTC. When I said you don't need to worry about time zones I meant as far as parsing the input. If a developer is planning to use unix time as an input I would expect them to understand how that works.

colinl commented 2 weeks ago

If a developer is planning to use unix time as an input I would expect them to understand how that works.

Remember that node-red is aimed at users that are not software developers. The inject node sends the current time, but if you feed that into the datetime node it would generally not display what the user considers to be the current time. Such issues cause endless confusion.