I'd extract 15 to the constant, for example DEFAULT_STEP_MINUTES=15

Good point, it was repeated in two places, so I've extracted it to constants 👍

What will happen if I do step=True or step="foo" or step=(90,) ?

I don't see any part of code that would actually check the type in runtime.

In most of our components we don't do such checking and just rely on Type Hints and runtime exceptions, in my opinion it'd be a bit of over engineering, however I can add some type checking if you insist. But in such case I guess we need some approval for further messages send to user from product, since this is outside of the spec.

I'm leaving this as is for now and waiting for a response from you

I'd say that all I'm asking boils down to changing your code from:

        if isinstance(step, timedelta):
            step = step.seconds
        if step < 60 or step > timedelta(hours=23).seconds:
            raise StreamlitAPIException(
                f"`step` must be between 60 seconds and 23 hours but is currently set to {step} seconds."
            )

to:

        if isinstance(step, timedelta):
            step = step.seconds
        elif not isinstance(step, int):
            raise StreamlitAPIException(f"`step` can only be `int` or `timedelta` but {type(step)} is provided")
        if step < 60 or step > timedelta(hours=23).seconds:
            raise StreamlitAPIException(
                f"`step` must be between 60 seconds and 23 hours but is currently set to {step} seconds."
            )

So we are talking about adding two lines of code :) IMO pretty cheap stuff to check, even if not done elsewhere.

Yes, it's cheap and as I wrote I'm not going to insist on that, thank you for the example and the message, I've updated it with a small change

        if not isinstance(step, (int, timedelta)):
            raise StreamlitAPIException(
                f"`step` can only be `int` or `timedelta` but {type(step)} is provided."
            )

this is called before any step conversion

Can we get these tests to use the new react testing library?

Yes, thank you for noticing this 🙇 , I've just updated the tests to use RTL instead of Enzyme, can you please re-review it? 👍

I think this test is specifically to test for ui changes. I am thinking that you can maybe call fireEvent here (https://testing-library.com/docs/dom-testing-library/api-events/) to test this.

Thank you for this suggestion, I'm a bit stuck, because I'm firing change event on baseui time input and seems like its not triggering setStringValue method, however when testing manually the timepicker works good 🤔

ditto