Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Synchronized Time Marker #4930

Closed
wants to merge 13 commits into from
Closed

Synchronized Time Marker #4930

wants to merge 13 commits into from

Conversation

kuy
Copy link

@kuy kuy commented Sep 12, 2015

Closes #3716.

This PR adds a synchronized time marker into time-based charts on dashboard. When you hover a mouse pointer on the data point of a chart, time markers are rendered on all time-based charts and track your cursor in real-time.

marker-sync

New components

  • MarkerSynchronizer

MarkerSynchronizer is a small service that receives a 'hover' event from Charts and emits a 'sync' event. This was introduced in order to avoid broadcasting of 'sync' events on $rootScope.

  • MarkerRenderer

MarkerRenderer encapsulates how to render time markers. This renderer is used by MarkerSynchronizer and TimeMarker.

Data/Event flow

[Chart(Vis)] --('hover' event)--> [MarkerSynchronizer]

[MarkerSynchronizer] --('sync' event)--> [Handler]

[Handler] --(render time marker)--> [Chart]

In the restoring process of savedVis object, MarkerSynchronizer subscribes hover event of vis object. Handler object subscribes sync event of MarkerSynchronizer and renders time marker using MarkerRenderer when sync event is fired.

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

@epixa
Copy link
Contributor

epixa commented Sep 12, 2015

@kuy Before this can be merged, you'll need to sign the Elastic CLA, which you can do online here: https://www.elastic.co/contributor-agreement/

@kuy
Copy link
Author

kuy commented Sep 12, 2015

@epixa Thanks for your guide. I've already signed CLA as "An individual contributor" and received PDF from Elastic and Adobe EchoSign. What should I do to pass below automatic CLA check?

@kuy
Copy link
Author

kuy commented Sep 12, 2015

@epixa Oops, it's passed. Thank you!

@rashidkpc
Copy link
Contributor

We should discuss if we want to continue on this path or if @stormpython will have the new visualization system ready shortly and if we should put effort into that instead. I don't want us to continue building on the current infrastructure if we're just planning to replace it anyway

@trevan
Copy link
Contributor

trevan commented Sep 14, 2015

@rashidkpc Can we get a preview of the new visualization system?

@rashidkpc
Copy link
Contributor

@kuy
Copy link
Author

kuy commented Sep 14, 2015

@rashidkpc Okay thanks. I wait response from @stormpython.

BTW, when I was working on this feature, I felt a gap between the actual visualization system and the code what I wanted to do. The visualization system is complicated, but it's just a renderer. This means it doesn't have a state basically and generated SVG elements are not reused by D3. On the other hand, to realize a synchronized time marker, I have to render frequently. It's possible in theory that constructing handler (or vis) object that contains positions of current time marker from scratch and generating SVG on every time a mouse cursor moves. But this causes performance issue obviously. So I hope the new visualization system considers this gap in order to handle a state in visualizations more smartly.

@stormpython
Copy link
Contributor

Yes, the current visualization system is complicated. Please see my comments in #3456. The new visualization system is going to be completely different from the current one. In the new visualization system, I imagine that this can be accomplished simply by passing a function to the hover event listener.

@stormpython
Copy link
Contributor

Since we are doing a complete rewrite of the visualization library, we don't want to invest in any new features in the current one. If you'd like to learn more about the future of the visualization library, you can watch for changes in: https://github.com/stormpython/jubilee and https://github.com/stormpython/phoenix

@rashidkpc
Copy link
Contributor

Since we're going another direction with the visualization library I'm going to close this

@rashidkpc rashidkpc closed this Oct 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants