onDateChange signature inconsistent with Semantic UI React components

[JonSilver]

🐛 bug report

Description of the problem

This component has a very different API/parameters/signature for onDateChange event to that presented by the Semantic UI React components. The components in Semantic UI React all have (event, data) signatures for event handlers such as those for onChange . This facilitates consistent treatment for the whole family of components using generic event handlers.

How has this issue affected you? What are you trying to accomplish?

The component simply doesn't play nicely in a form where generic event handlers have been written to interact with the events of Semantic UI React components.

[arthurdenner]

Hi @JonSilver! 👋

I think you have a great point, would be nice to match the APIs.

Would you be interested to work on this?

[JonSilver]

Thanks @arthurdenner. I wouldn't say no, but I have precious little time for my own projects, let alone others'. However I definitely have a use for such a component or I wouldn't have found yours. 🙂

There would be significant breaking changes to the API which would at least inconvenience existing users. Or we could leave onDateChange as it is, and also implement the more standard onChange event with the Semantic UI React standard function signature. Call both events on an update and everyone's happy.
It would also be great if we could have a value property for the selected date instead of the non-standard selected.

My main focus here would be on standardising APIs - almost everything else about your component is great (maybe apart from the external CSS dependency) 🙂. I've had a look at a couple of such components - the other one makes a date picker component synchronise with a string value rather than a JS date, which is just weird, not friendly for multi-locale usage, and incredibly inconvenient. But it does do times, which yours doesn't - and could be useful.

[arthurdenner]

I don't mind having breaking changes. That would cause a major version bump anyway, so users would be aware of the changes and would migrate if they think it's necessary.

I don't like the idea of having two methods, I'd rather rename onDateChange and change its signature and rename the selected prop too.

I'll flag this as help wanted because I'm currently busy as well and the changes are not necessary for my use cases. If I find time to working on it later, I will, but anyone is welcome to implement. 👋

Thank you for the suggestions again, they are very great.

[JonSilver]

Ok @arthurdenner I'll probably end up forking your repo and making the modifications for myself anyway, so if I do I'll submit a PR once I've tested it all. Thanks for being so receptive to my suggestions. 🙂

[arthurdenner]

🎉 This issue has been resolved in version 2.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀