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

Brush onBrushEnd does not seem to report the end x position #1789

Open
holgergp opened this issue Jan 18, 2024 · 2 comments
Open

Brush onBrushEnd does not seem to report the end x position #1789

holgergp opened this issue Jan 18, 2024 · 2 comments

Comments

@holgergp
Copy link

holgergp commented Jan 18, 2024

Hello visx folks!

I am currently introducing a brush to a bar char.
While working quite nicely, I noticed that the onBrushEnd callback - that reports a bound - does not report the target x value correctly.
I made a sandbox for this: https://codesandbox.io/p/sandbox/relaxed-star-q6jjzg?file=%2Fsrc%2FApp.js
There I introduced a Brush which is console logging the bound onBrushEnd.
The Brush is using a scaleBand on the x-axis and a scaleLinear on the y-axis.
The bound which is reported has the following structure:
{ "x0": 0, "x1": 0, "xValues": [ "Item 1", "Item 2" ], "y0": -31.428571428571406, "y1": 402.85714285714283 }

While y0 and y1 seem reasonable, x0 might be correct, x1 being 0 is not what I am expecting, as the rectangle has width > 0.

Am I using the brush wrong? Looks like a bug to me.
How would the idea be, to get the coordinates of the brushed rectangle?

Thanks for your awesome work!

Cheers from Düsseldorf!

Holger

@holgergp holgergp changed the title Brush onBrush does not seem to report the end x position Brush onBrushEnd does not seem to report the end x position Jan 18, 2024
@dsdevgit
Copy link
Contributor

dsdevgit commented Jan 31, 2024

Hi there.

Just some thoughts on potential bug:

The scaleBand type of scale doesn't have the invert() function we need to convert pixel space to values space (see https://d3js.org/d3-scale/band

As a result, the bounds struct will contain x as zero (if undefined, etc.).
see Brush.tsx (convertRangeToDomain())

/// Brush/Brush.tsx

convertRangeToDomain(brush: BaseBrushState) {
    const { xScale, yScale } = this.props;
    const { x0, x1, y0, y1 } = brush.extent;

    const xDomain = getDomainFromExtent(xScale, x0 || 0, x1 || 0, SAFE_PIXEL);
    const yDomain = getDomainFromExtent(yScale, y0 || 0, y1 || 0, SAFE_PIXEL);

    const domain: Bounds = {
      x0: xDomain.start || 0,
      x1: xDomain.end || 0,
      xValues: xDomain.values,
      y0: yDomain.start || 0,
      y1: yDomain.end || 0,
      yValues: yDomain.values,
    };

    return domain;
  }

As I see it, the scaleInvert function wraps cases like this but seems to be the code is focused on ordinalScale only.
see utils.tsx (getDomainFromExtent(), scaleInvert())

May it have the bug?

PS. at the moment I am unable to debug that...sorry
PS. The idea by holgergp to extend the bounds with extra info looks great for me.
If I remember, the d3-brush events contain target object etc as well.

Regards,
Den

@nd-novorender
Copy link

nd-novorender commented Jun 12, 2024

@dsdevgit I also get wrong wrong domain report in the end - it's slightly extended (by a couple of pixel).
In onBrushEnd I update original value and then update brush value with updateBrush.
For example if I only move the right boundary - the left one also shifts. Looks like it's happening because of SAFE_PIXEL. Can you explain what's the purpose of it? If I set it to 0 - there are no issues.

Here's the reproduction https://codesandbox.io/p/sandbox/quizzical-khorana-yf9dkw?file=%2FExample.tsx%3A130%2C20 - just click on the brush or try to move/resize.

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