Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

VictoryZoomContainer - hard to use with real-time / dynamic data #646

Closed
chrisbolin opened this issue Jul 10, 2017 · 15 comments · Fixed by FormidableLabs/victory-chart#496
Closed
Assignees
Labels
Type: Performance 📈 Issues related to performance

Comments

@chrisbolin
Copy link
Contributor

demo: https://jsfiddle.net/chrisbolin/hjy9v9z4/

Usually when using Victory with real-time datasets (i.e. that grow over time) the visible domain will keep "growing" to accommodate new data.

However, once you zoom the first time with VictoryZoomContainer, you cannot get back to the state where the visible domain automatically grows. No matter how much you zoom out, you are still stuck with static view.

jul-10-2017 17-26-36 zoom real time

@chrisbolin chrisbolin self-assigned this Jul 10, 2017
@chrisbolin
Copy link
Contributor Author

cc @arwinsardana, who found this and posted it to gitter

@chrisbolin
Copy link
Contributor Author

also, resetting the zoomDomain to null after a full zoom out doesn't fix the problem: https://jsfiddle.net/chrisbolin/hjy9v9z4/4/

@chrisbolin
Copy link
Contributor Author

@arwinsardana, i want to make sure I don't assume too much -- could you tell me how you envision VictoryZoomContainer working in this scenario? (a detailed step-by-step would be helpful, e.g. "the user zooms in and the chart does ____")

@chrisbolin
Copy link
Contributor Author

@boygirl I'd also love your thoughts - how "should" the chart react in situations like this: https://jsfiddle.net/chrisbolin/hjy9v9z4/

@arwinsardana
Copy link

arwinsardana commented Jul 12, 2017

@chrisbolin thanks for following up!

When the user zooms in, I'd imagine the domain stays fixed and everything works the same as it would with a fixed data set. In other words, once zoomed in to a range smaller than the full chart's domain, that view stays fixed until further action.

When you pan to the right edge of the chart while zoomed in, I'm not sure if the behavior should be that the window stays where it is or slides as new data comes in. I'd be fine with the former, static window, approach when in a zoomed state.

Once the user zooms out to the point where the current domain is equivalent to the full domain of the chart, the container should update itself to display the full domain going forward (until another zoom event). In other words, once fully zoomed out, the chart should appropriately display new data points as it would without ever "remembering" that the user had zoomed in, just as it would be displaying without the zoom feature enabled.

Let me know if I can provide any more details and thanks again!

@boygirl
Copy link
Contributor

boygirl commented Jul 12, 2017

@chrisbolin I think the behavior should be:

  1. When zoomed in, or zooming in the domain remains static
  2. When zooming out, or panning, the domain is calculated.

Does that sound right?

We will need to re-calculate the domain much more frequently, and not used the cached originalDomain for setting domain values. I'm not sure how much of a performance trade off this will be, but I think it's worth a shot.

@chrisbolin
Copy link
Contributor Author

@boygirl @arwinsardana cool! I think we all agree then on the functionality. I'll see what I can put together.

@arwinsardana
Copy link

@chrisbolin I've noticed that the y domain doesn't update on zoom out. Here's your example modified to start with less initial data and draw slower: https://jsfiddle.net/kfy2ce5m/

If you zoom in and out before a full cycle of the sin wave, you'll notice the full wave can never be seen because the y domain doesn't correct itself. Thoughts? Thanks! CC @boygirl

@boygirl boygirl reopened this Sep 6, 2017
@arwinsardana
Copy link

Hey @chrisbolin! Let me know if you have any thoughts. Thanks!

@chrisbolin
Copy link
Contributor Author

hey @arwinsardana! sorry for my tardiness! let me quickly check this out tomorrow and get back to you

@chrisbolin
Copy link
Contributor Author

good morning! it looks like are "reset" logic that looks to see if we are zoomed in or not needs to be reworked. It also needs to respect the dimension prop. I don't have to do make the changes right now, but if you want to work on it I can show you the section of code.

@chrisbolin
Copy link
Contributor Author

also, i cannot get this to happen if I remove dimension="x". Is that the case for you? If so, we might have a super hacky workaround. Also, that fact might tell us something about the solution too.

@ryan-roemer ryan-roemer added the Type: Performance 📈 Issues related to performance label Feb 23, 2018
@caseyjkey
Copy link

Have changes been implemented to allow for zooming in and allows the domain to automatically scroll right as new data comes in?

@becca-bailey
Copy link
Contributor

@boygirl Do you know anything about the status of this one?

@boygirl
Copy link
Contributor

boygirl commented Nov 2, 2021

I believe this is still an issue

@FormidableLabs FormidableLabs locked and limited conversation to collaborators Feb 14, 2022
@becca-bailey becca-bailey converted this issue into discussion #2098 Feb 14, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Type: Performance 📈 Issues related to performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants