SceneTimeRange: onTimeZoneChange and onTimeRangeChange to set

[tskarhed]

This step will help a little bit, but I think I want to make another PR to change these to setTimeZone and setTimeRange.

Here is my suggestion for doing so, please give me your thoughts:

1. Mark these as deprecated
2. Add setTimeZone and setTimeRange.
3. Call the new methods from the deprecated methods
4. Update unit test with new methods

Then it is possible to do a gradual change. Thoughts?

[dprokop]

Thanks @tskarhed . This sounds like a good plan! Mind adding docs as well for what you've just learned about setting time range externally? Think adding it to https://grafana.com/developers/scenes/advanced-data#use-time-range would make sense.

[torkelo]

would this not break the guideline we have for react callback handlers to begin with onXX ? to match when we hook them up like this.

I do agree that this naming does make it feel like something you should maybe not call manually (outside a react callback handler).

Do we have any other onXX interface/ scene object methods in scenes (want to make sure we do not add some inconsistency here). I think MultiValueVariable has changeValueTo (so not using onXX).

Not seeing and change to stop using the deprecated versions here

[tskarhed]

Use dateTime when it is an epoch

[tskarhed]

would this not break the guideline we have for react callback handlers to begin with onXX ? to match when we hook them up like this.

I do agree that this naming does make it feel like something you should maybe not call manually (outside a react callback handler).

Do we have any other onXX interface/ scene object methods in scenes (want to make sure we do not add some inconsistency here). I think MultiValueVariable has changeValueTo (so not using onXX).

Not seeing and change to stop using the deprecated versions here

Hmm, I see your point. Would it make sense for these two to just exist in parallel, just for the sake of good naming? You would use setTimeRange whenever you need to do it manually, and onTimeRangeChange for callback handling.

Maybe that would add complexity when implementing the interface though.

[dprokop]

Hmm, I see your point. Would it make sense for these two to just exist in parallel, just for the sake of good naming? You would use setTimeRange whenever you need to do it manually, and onTimeRangeChange for callback handling.

@torkelo @tskarhed I don't think having two makes sense. API is one thing, the callback is another. For the SceneTimePicker and any other usages that are callbacks the model can wrap the API in a higher-level function that will follow the on... convention. But since we are talking about API that can be used in multiple contexts, I think we need to name it in a way that is understandable and context agnostic. With that in mind I would suggest callin it updateTimeRange or setTimeRange and follow this for all the public model methods that can be used in an ambiguous way.

[torkelo]

@dprokop yea, agree.