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

Victory Brush/Zoom out of sync #2023

Closed
3 tasks done
jyshin97 opened this issue Nov 10, 2021 · 2 comments
Closed
3 tasks done

Victory Brush/Zoom out of sync #2023

jyshin97 opened this issue Nov 10, 2021 · 2 comments

Comments

@jyshin97
Copy link

jyshin97 commented Nov 10, 2021

Bugs and Questions

Checklist

  • I have read through the FAQ and Guides before asking a question

  • I am using the latest version of Victory

  • I've searched open issues to make sure I'm not opening a duplicate issue

The Problem

There's a previous issue related to this (over a year old) but hasn't been addressed yet. In a combined Victory Zoom/Brush example, the brush goes out of sync with the zoom when you adjust the brush to a smaller value and then zoom back out using a mouse scroll.

Reproduction

https://codesandbox.io/s/muddy-architecture-2fqbu?file=/index.js

Based on the brush/zoom example in the docs.

  1. Set the brush to any length on either side smaller than the start
  2. Zoom out using the mouse wheel

My (very naive) guess as to what's happening here after looking into the source code is that some of the values are not being updated/re-run as needed since it's the zoom that's controlling the domain. It seems that currentDomain is being set when the brush is being configured (as expected) but it's not being re-calc'd on zoom. Then when it's running into the following code when we zoom back out

    getRect(props) {
      const { currentDomain, cachedBrushDomain } = props;
      const brushDomain = defaults({}, props.brushDomain, props.domain);
      const domain = isEqual(brushDomain, cachedBrushDomain)
        ? defaults({}, currentDomain, brushDomain)
        : brushDomain;
      const coordinates = Selection.getDomainCoordinates(props, domain);
      const selectBox = this.getSelectBox(props, coordinates);
      return selectBox ? [selectBox, this.getHandles(props, domain)] : [];
    }

it's setting the domain to be the currentDomain which hasn't been updated (I believe in this case cachedBrushDomain is the entire domain since this only happens when zoomDomain returns to its original value. This could be way out of the ballpark but just a best guess. Please let me know if there's a potential workaround/fix coming soon for this!

@AaronPlave
Copy link

I'm also running into this issue – has anyone found a workaround?

@becca-bailey
Copy link
Contributor

Closing in favor of #2111

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

No branches or pull requests

3 participants