Shmew / Feliz.Plotly

Fable bindings written in the Feliz-style for plotly.js.
https://shmew.github.io/Feliz.Plotly/
MIT License
54 stars 13 forks source link

Add support for DateTimeOffset #26

Closed nojaf closed 3 years ago

nojaf commented 3 years ago

Is your feature request related to a problem?

Hello, would it be possible to add support of DateTimeOffset as well? image

Describe the solution you'd like

Out of the box support would be nice.

Describe alternatives you've considered

I probably can convert my DateTimeOffset to a DateTime, though I'm not really sure how to proceed there.

Additional context

/

Zaid-Ajaj commented 3 years ago

I probably can convert my DateTimeOffset to a DateTime, though I'm not really sure how to proceed there.

You can get the DateTime component using the DateTime property

let timestamp = DateTimeOffset.Now;
let date = timestamp.DateTime;

In Fable, both DateTimeOffset and DateTime are Date instances so you could even unbox<DateTime>. The only difference is that DateTimeOffset attaches a property to the Date instance to maintain the offset

nojaf commented 3 years ago

You lose the time information when calling timestamp.DateTime;. https://fable.io/repl/#?code=PTAEHUCcEsBcFNQGMD2ATRLKgDYoIZqj6gDO+AtgA46IBmkKFZ0GARvpALABQKV8AHagAygE9SCCr14ARfAgAq0CvADydOqXiwAdAAVO2gBQAiAEwAGAIwBOALSWAzPesBWUJYDsALjfmfABZrXSd3SwjTAEpdeQReAB8APlAqGEFYOmFTAFIAQVMgA&html=Q&css=Q

Zaid-Ajaj commented 3 years ago

The REPL snippet is calling .Date, not .DateTime which only returns the date component

nojaf commented 3 years ago

😮, ok my bad. Thanks for pointing this out Zaid!

Zaid-Ajaj commented 3 years ago

😉 No problem! I think you can leave the issue open, it would be a nice addition to have an overload that accepts DateTimeOffset[] and then convert to DateTime internally

Shmew commented 3 years ago

I just pushed version 2.1.0 that adds support for DateTimeOffset anywhere a DateTime was previously allowed. Let me know if you have any issues, and feel free to re-open this if so!

nojaf commented 3 years ago

Thanks, I've tested this and it works as expected.