visx icon indicating copy to clipboard operation
visx copied to clipboard

initialBrushPosition and the first call to update the brush are slightly out of sync

Open JoeVanGundy opened this issue 3 years ago • 4 comments

Update: have since discovered this is actually a bug in Visx (or unexpected behavior)

initialBrushPosition and the brush seem to be slightly out of sync. The first time the brush is used after a initialBrushPosition has been set,

Reproduction steps:

  1. Set the brush with initialBrushPosition
  2. Observe the values that are being set (Mon Aug 15 2011 00:00:00 GMT-0700 (Pacific Daylight Time))
  3. Move the left side of the brush to an earlier date.
  4. Notice how the right side of the brush is updated when it's not suppose to be: (Tue Aug 16 2011 06:11:10 GMT-0700 (Pacific Daylight Time))

Causing lots of issues for us since we're using the brush for day selection.

Is this a known issue or expected?

Here are steps to reproduce the bug in the base brush example: https://codesandbox.io/s/hopeful-cache-xu025?file=/Example.tsx

JoeVanGundy avatar Sep 15 '21 05:09 JoeVanGundy

Hello @hshoff @williaster ,

actually the values are not slightly out of sync, they are out of scale and domain.

In the demo open the console and click the brush selection. You'll see that zero value is not 0 it shows -2.000000000542981, more over the last position is higher than xMax value. See the screenshot.

Could you explain this behavior and how to fix it?

Thank you!

image

alexandrbig avatar May 10 '22 12:05 alexandrbig

@JoeVanGundy I managed to fix it by using new xScale for brush only:

const xScaleBrush = useMemo(
    () =>
      scaleTime({
        domain: xScale.domain(),
        // https://github.com/airbnb/visx/blob/master/packages/visx-brush/src/Brush.tsx#L119
        // visx brush adds 2 pixels for some reason to the origin range
        range: [-2, xMax + 2],
      }),
    [xScale, xMax],
  );

Anyhow this addition looks strange, there is no explanation why and for what. I can only assume that it is because of the stroke width of the brush rect. But i might be wrong.

alexandrbig avatar May 10 '22 13:05 alexandrbig

Unfortunately the fix provided above doesn't fix the whole issue with this "additions" as we have the range 4px wider for the brush in comparison to a normal scale.

alexandrbig avatar May 10 '22 16:05 alexandrbig

Currently fixed this issue in a following way:

const brushSafeDiff = useMemo(() => {
    // Brush adds 2 pixels to the xScale range, we have to calculate this diff beforehand to extract/add it to bounds
    return xScale.invert(2).getTime() - xScale.domain()[0].getTime();
  }, [xScale])

and then in onChange method:

onChange = (bounds) => {
      if (bounds) {
        const xBounds = [
          Math.max(bounds.x0 + brushSafeDiff, xScale.domain()[0].getTime()),
          Math.min(bounds.x1 - brushSafeDiff, xScale.domain()[1].getTime())
        ] as [number, number];
...

So we extract and add these 2 px each time when we get brush bounds.

alexandrbig avatar May 11 '22 08:05 alexandrbig