[RFC][Maps] Adding a timeslider to Maps#98355
[RFC][Maps] Adding a timeslider to Maps#98355thomasneirynck merged 10 commits intoelastic:masterfrom
Conversation
nreese
left a comment
There was a problem hiding this comment.
Thanks for putting this together. Its great and really captures timeslider and all of its details.
|
Pinging @elastic/kibana-gis (Team:Geo) |
|
Thanks for making the changes that we discussed on Slack. It looks good! 🎉 |
nreese
left a comment
There was a problem hiding this comment.
just some minor suggestions.
LGTM
| - This would require the extraction of the timeslider React-component out of Maps into a separate plugin. As outlined above, this migration should be fairly straightforward, a "cut and paste". | ||
| - It would require the creation of a `TimesliderEmbeddable` which wraps this UI-component. | ||
| - It would also require the introduction of a new piece of embeddable-state, `Timeslice`, which can be controlled by the `TimesliderEmbeddable`. | ||
| We believe it is important to keep `timeslice` and `timerange` separate, as individual apps and other embeddables will have different mechanism to efficiently fetch data and respond to changes in `timeslice` and/or `timerange`. |
There was a problem hiding this comment.
I'm not entirely convinced of this, but I don't think it has any effect on this RFC, which looks great to me. Since I don't think the current implementation plan will back us into any corners one way or the other, LGTM!
There was a problem hiding this comment.
thx @stacey-gammon
Yes, we can spin-off the discussion of adding new embeddable state. It agree it might not be necessary from a high level (e.g. timeslider as an Embeddable).
The reason we raised it here, is because keeping timeslice and timerange separate does solve a particular problem for Maps. It allows some layer-types to prefetch data: ie. get all the data within the entire timerange up-front, and then do client-side filtering to respond to timeslice-changes. This avoids round-trips to Elasticsearch. But this is definitely an optimization.
|
LGTM |
Yeah @AlonaNadler. We're planning incremental improvements for the timeslider. Current milestones we're planning right now look like these:
|
|
We're going to close this. Thx for all the feedback - on and offline. @flash1293 had prepared an outline for how a timeslider could behave on a dashboard, and the implications wrt implementing this. https://gist.github.com/flash1293/3a9cf36c331e3f59ed85f0843e68f94e It makes roughly a similar determination for dashboards as we're doing here for Maps:
@flash1293 also proposed a "third" "hardest" way, which would combine caching and pre-fetching somewhat "automagically". That is not something we're entertaining short term for the Maps implementation. For completeness, wrt prefetching: The first PR for the Maps timeslider, #99661, removes any pre-fetching logic. This because we are running in more edge-cases than anticipated. We're hoping to dial in prefetch optimizations at a later time. |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
RFC for adding Timeslider to Maps.
A timeslider is a long-standing feature request for the Maps application: #27714
A working end-2-end POC is available here: #96791
We are looking to gather feedback since a timeslider would be more broadly useful in other Kibana apps as well.
Please comments on the doc https://github.com/elastic/kibana/pull/98355/files#diff-215d9ecb40e59418874139a12cb2fbde9d15d51dad380f0c947119b606980e7d . Depending on what the kind of questions and scope of feedback, we might spin these off in a separate Github discussion.
Thanks!