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

Weird zoom in/out! #2761

Open
omid opened this issue Jan 31, 2024 · 18 comments
Open

Weird zoom in/out! #2761

omid opened this issue Jan 31, 2024 · 18 comments
Labels
Status: Needs More Info ✋ A question or report that needs more info to be addressable

Comments

@omid
Copy link

omid commented Jan 31, 2024

Describe the bug
You can zoom out out of the chart, and the zoom out is weird then!! I don't know how to describe it. watch the screencast below!

Victory version
Your version at: https://formidable.com/open-source/victory/docs/victory-zoom-container/

Code Sandbox link
Your online example: https://formidable.com/open-source/victory/docs/victory-zoom-container/

To Reproduce
First, zoom in, and then zoom out many times, out of the chart. Like the screencast here.

Peek.2024-01-31.10-30.mp4

Desktop (please complete the following information):

  • OS: Linux
  • Browser: Chrome/Firefox
  • Version: latest of each
@carbonrobot
Copy link
Contributor

The zoom is relative to your cursor position, try zooming out / in with your cursor centered in the graph

@omid
Copy link
Author

omid commented Jan 31, 2024

@carbonrobot first of all, why can I zoom off the chart?
second, it's not about that, watch the screencast more carefully, when I am off the chart, I ONLY zoom out, but the chart is in a loop of zoom-in/out!

@carbonrobot
Copy link
Contributor

carbonrobot commented Jan 31, 2024

Your cursor appears to be still over the chart. Remember, the chart has padding all the way around and fills the entire column.

watch the screencast more carefully

Obviously, it is unclear what your mouse is doing, because we can't see what buttons are being pressed via the video.

I am unable to replicate that behaviour using Edge/Chrome/Firefox/Safari.

@omid
Copy link
Author

omid commented Jan 31, 2024

Sure you cannot see what I do with my mouse buttons :D But I'm telling you what I'm doing.

Steps to reproduce:

  1. Go inside and zoom in
  2. Go in the white part, out of the chart and zoom out a lot

@carbonrobot
Copy link
Contributor

Everything appears to be working as intended. The zoom is correct relative to the mouse position in the data points.

Could you describe the behavior you are expecting?

zoom

@omid
Copy link
Author

omid commented Jan 31, 2024

what you have done is correct, but zoom out more than that, don't stop zooming out.
to face the issue earlier, you don't need to zoom in a lot, one step is enough.

@carbonrobot
Copy link
Contributor

I'm still unclear what you are asking. Are you talking about the numbers repeating due to the datum using Math.Pi and Sin?

@omid
Copy link
Author

omid commented Jan 31, 2024

It's generally about both axises, but in the video you can see x-axis.
In the video below, I ONLY ONCE, scroll up (zoom-in) and after that JUST scroll down (zoom-out).
The scroll down is happening FIRST out of the chart THEN inside and outside of the chart.

Peek.2024-01-31.14-03.mp4

@carbonrobot
Copy link
Contributor

The video looks normal to me, I'm still unclear on the question.

Here is the charts bounding box (it receives mouse events inside this red border)

image

Could you describe the problem with the following format?

  1. Test steps
  2. Expected result
  3. Actual result

@omid
Copy link
Author

omid commented Jan 31, 2024

I used this script to show my mouse events :D
window.addEventListener("wheel", event => console.log(event.wheelDelta > 0 ? "scroll-up (zoom-in)" : "scroll-down (zoom-out)"));

Steps:

  • ONE zoom in, inside the chart
  • MANY zoom outs in the white area (actually zoom out kinda zoom in)

In the video below, there is ONLY ONE zoom in, and after that, all others are zoom out!

Peek.2024-01-31.16-19.mp4

@carbonrobot
Copy link
Contributor

Mouse wheel events have a deltaX, deltaY, and deltaZ property. I think the code you are looking for is

window.addEventListener("wheel", event => console.log(event.deltaY > 0 ? "scroll-up (zoom-in)" : "scroll-down (zoom-out)")); 

@omid
Copy link
Author

omid commented Jan 31, 2024

Don't debug my code snippet, debug the library :D
In both cases, the bug is there and is the same.

@carbonrobot
Copy link
Contributor

carbonrobot commented Jan 31, 2024

Do you see the problem in this screen cap? I'm not seeing anything incorrect, but maybe we are talking about different things.

zoom2

@omid
Copy link
Author

omid commented Jan 31, 2024

yes, I see that :)

when you did 26 scroll-ups, it means ZOOM OUT... but actually the chart zooming in.

To see the bug better, never do scroll-down after the first one. do scroll-down JUST ONCE

@carbonrobot
Copy link
Contributor

I'm not seeing a bug, I'm just seeing the designed behavior. Can you describe the actual VS expected behavior?

@carbonrobot carbonrobot added Status: Needs More Info ✋ A question or report that needs more info to be addressable Triage labels Jan 31, 2024
@carbonrobot
Copy link
Contributor

Perhaps there is some confusion in this example because it uses an initial zoom domain (in other words, the charts starts pre-zoomed-in).

You can paste the following code into the "Editable Source" box to see if that changes anything for you.

class App extends React.Component {
  constructor(props) {
    super(props);
  }
  state = {
    data: this.getScatterData()
  }
  getScatterData() {
    return range(50).map((index) => {
      return {
        x: random(1, 50),
        y: random(10, 90),
        size: random(8) + 3
      };
    });
  }
  render() {
    return (
      <VictoryChart
        containerComponent={<VictoryZoomContainer/>}
      >
        <VictoryScatter
          data={this.state.data}
          style={{
            data: {
              opacity: ({ datum }) =>  datum.y % 5 === 0 ? 1 : 0.7,
              fill: ({ datum }) => datum.y % 5 === 0 ? "tomato" : "black"
            }
          }}
        />
      </VictoryChart>
    );
  }
}
ReactDOM.render(<App/>, mountNode)

I think we are more than happy to change something, we just need to know what you would like changed.

@omid
Copy link
Author

omid commented Jan 31, 2024

same :/

Peek.2024-01-31.18-16.mp4

@omid
Copy link
Author

omid commented Feb 2, 2024

@carbonrobot can we meet somewhere? google meet or discord or wherever you can?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs More Info ✋ A question or report that needs more info to be addressable
Projects
None yet
Development

No branches or pull requests

2 participants