Merge pull request #667 from hshoff/chris--initialbrushposition-fix

fix(vx-brush): Fix initialBrushPosition
airbnb · May 2, 2020 · 8e35362 · 8e35362
2 parents 5fa4fbf + 133fcaa
commit 8e35362
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 64 deletions.
diff --git a/packages/vx-brush/src/BaseBrush.tsx b/packages/vx-brush/src/BaseBrush.tsx
@@ -6,7 +6,7 @@ import Drag, { HandlerArgs as DragArgs } from '@vx/drag/lib/Drag';
 import BrushHandle from './BrushHandle';
 import BrushCorner from './BrushCorner';
 import BrushSelection from './BrushSelection';
-import { MarginShape, Point, BrushShape, BrushStartEnd, ResizeTriggerAreas } from './types';
+import { MarginShape, Point, BrushShape, ResizeTriggerAreas, PartialBrushStartEnd } from './types';
 
 const BRUSH_OVERLAY_STYLES = { cursor: 'crosshair' };
 
@@ -16,7 +16,7 @@ type MouseHandlerEvent =
 
 export type BaseBrushProps = {
   brushDirection?: 'horizontal' | 'vertical' | 'both';
-  initialBrushPosition?: BrushStartEnd;
+  initialBrushPosition?: PartialBrushStartEnd;
   width: number;
   height: number;
   left: number;
@@ -49,18 +49,19 @@ export type UpdateBrush =
 export default class BaseBrush extends React.Component<BaseBrushProps, BaseBrushState> {
   private constructor(props: BaseBrushProps) {
     super(props);
+    const { initialBrushPosition } = props;
+    const extent = initialBrushPosition
+      ? this.getExtent(initialBrushPosition.start, initialBrushPosition.end)
+      : {
+          x0: -1,
+          x1: -1,
+          y0: -1,
+          y1: -1,
+        };
     this.state = {
-      start: { x: 0, y: 0 },
-      end: { x: 0, y: 0 },
-      ...this.props.initialBrushPosition,
-      extent: this.props.initialBrushPosition
-        ? this.getExtent(this.props.initialBrushPosition.start, this.props.initialBrushPosition.end)
-        : {
-            x0: -1,
-            x1: -1,
-            y0: -1,
-            y1: -1,
-          },
+      start: { x: Math.max(0, extent.x0), y: Math.max(0, extent.y0) },
+      end: { x: Math.max(0, extent.x1), y: Math.max(0, extent.y1) },
+      extent,
       bounds: {
         x0: 0,
         x1: this.props.width,
@@ -98,14 +99,6 @@ export default class BaseBrush extends React.Component<BaseBrushProps, BaseBrush
     initialBrushPosition: null,
   };
 
-  componentDidMount() {
-    window.addEventListener('mouseup', this.handleDragEnd);
-  }
-
-  componentWillUnmount() {
-    window.removeEventListener('mouseup', this.handleDragEnd);
-  }
-
   componentDidUpdate(prevProps: BaseBrushProps) {
     if (this.props.width !== prevProps.width || this.props.height !== prevProps.height) {
       // eslint-disable-next-line react/no-did-update-set-state
@@ -120,12 +113,12 @@ export default class BaseBrush extends React.Component<BaseBrushProps, BaseBrush
     }
   }
 
-  getExtent = (start: Point, end: Point) => {
+  getExtent = (start: Partial<Point>, end: Partial<Point>) => {
     const { brushDirection, width, height } = this.props;
-    const x0 = brushDirection === 'vertical' ? 0 : Math.min(start.x, end.x);
-    const x1 = brushDirection === 'vertical' ? width : Math.max(start.x, end.x);
-    const y0 = brushDirection === 'horizontal' ? 0 : Math.min(start.y, end.y);
-    const y1 = brushDirection === 'horizontal' ? height : Math.max(start.y, end.y);
+    const x0 = brushDirection === 'vertical' ? 0 : Math.min(start.x || 0, end.x || 0);
+    const x1 = brushDirection === 'vertical' ? width : Math.max(start.x || 0, end.x || 0);
+    const y0 = brushDirection === 'horizontal' ? 0 : Math.min(start.y || 0, end.y || 0);
+    const y1 = brushDirection === 'horizontal' ? height : Math.max(start.y || 0, end.y || 0);
 
     return {
       x0,
@@ -163,19 +156,18 @@ export default class BaseBrush extends React.Component<BaseBrushProps, BaseBrush
     }));
   };
 
-  handleDragMove = (draw: DragArgs) => {
+  handleDragMove = (drag: DragArgs) => {
     const { left, top, inheritedMargin } = this.props;
-    if (!draw.isDragging) return;
+    if (!drag.isDragging) return;
     const marginLeft = (inheritedMargin && inheritedMargin.left) || 0;
     const marginTop = (inheritedMargin && inheritedMargin.top) || 0;
     const end = {
-      x: (draw.x || 0) + draw.dx - left - marginLeft,
-      y: (draw.y || 0) + draw.dy - top - marginTop,
+      x: (drag.x || 0) + drag.dx - left - marginLeft,
+      y: (drag.y || 0) + drag.dy - top - marginTop,
     };
     this.updateBrush((prevBrush: BaseBrushState) => {
       const { start } = prevBrush;
       const extent = this.getExtent(start, end);
-
       return {
         ...prevBrush,
         end,
@@ -185,36 +177,33 @@ export default class BaseBrush extends React.Component<BaseBrushProps, BaseBrush
   };
 
   handleDragEnd = () => {
-    const { isBrushing } = this.state;
-    if (isBrushing) {
-      const { onBrushEnd, resetOnEnd } = this.props;
-      this.updateBrush((prevBrush: BaseBrushState) => {
-        const { extent } = prevBrush;
-        const newState = {
-          ...prevBrush,
-          start: {
-            x: extent.x0,
-            y: extent.y0,
-          },
-          end: {
-            x: extent.x1,
-            y: extent.y1,
-          },
-          isBrushing: false,
-          activeHandle: null,
-        };
+    const { onBrushEnd, resetOnEnd } = this.props;
+    this.updateBrush((prevBrush: BaseBrushState) => {
+      const { extent } = prevBrush;
+      const newState = {
+        ...prevBrush,
+        start: {
+          x: extent.x0,
+          y: extent.y0,
+        },
+        end: {
+          x: extent.x1,
+          y: extent.y1,
+        },
+        isBrushing: false,
+        activeHandle: null,
+      };
 
-        if (onBrushEnd) {
-          onBrushEnd(newState);
-        }
+      if (onBrushEnd) {
+        onBrushEnd(newState);
+      }
 
-        if (resetOnEnd) {
-          this.reset();
-        }
+      if (resetOnEnd) {
+        this.reset();
+      }
 
-        return newState;
-      });
-    }
+      return newState;
+    });
   };
 
   getBrushWidth = () => {

diff --git a/packages/vx-brush/src/Brush.tsx b/packages/vx-brush/src/Brush.tsx
@@ -1,6 +1,13 @@
 import React from 'react';
 import BaseBrush, { BaseBrushProps, BaseBrushState } from './BaseBrush';
-import { Bounds, BrushStartEnd, MarginShape, Point, ResizeTriggerAreas, Scale } from './types';
+import {
+  Bounds,
+  PartialBrushStartEnd,
+  MarginShape,
+  Point,
+  ResizeTriggerAreas,
+  Scale,
+} from './types';
 import { scaleInvert, getDomainFromExtent } from './utils';
 
 const SAFE_PIXEL = 2;
@@ -20,7 +27,7 @@ export type BrushProps = {
   onClick: BaseBrushProps['onClick'];
   margin: MarginShape;
   brushDirection: 'vertical' | 'horizontal' | 'both';
-  initialBrushPosition?: BrushStartEnd;
+  initialBrushPosition?: PartialBrushStartEnd;
   resizeTriggerAreas: ResizeTriggerAreas[];
   brushRegion: 'xAxis' | 'yAxis' | 'chart';
   yAxisOrientation: 'left' | 'right';

diff --git a/packages/vx-brush/src/types.ts b/packages/vx-brush/src/types.ts
@@ -29,6 +29,11 @@ export interface BrushStartEnd {
   end: Point;
 }
 
+export interface PartialBrushStartEnd {
+  start: Partial<Point>;
+  end: Partial<Point>;
+}
+
 export type ResizeTriggerAreas =
   | 'left'
   | 'right'

diff --git a/packages/vx-demo/src/components/Gallery.tsx b/packages/vx-demo/src/components/Gallery.tsx
@@ -67,7 +67,7 @@ const items = [
 const tiltOptions = { max: 8, scale: 1 };
 const detailsHeight = 76;
 
-export default function() {
+export default function Gallery() {
   return (
     <div>
       <div className="gallery">

diff --git a/packages/vx-demo/src/components/tiles/Brush.tsx b/packages/vx-demo/src/components/tiles/Brush.tsx
@@ -16,7 +16,7 @@ import { ShowProvidedProps, MarginShape } from '../../types';
 /**
  * Initialize some variables
  */
-const stock = appleStock.slice(1200);
+const stock = appleStock.slice(1000);
 const axisColor = '#fff';
 const axisBottomTickLabelProps = {
   textAnchor: 'middle' as const,
@@ -155,6 +155,11 @@ function BrushChart({
     nice: true,
   });
 
+  const initialBrushPosition = {
+    start: { x: brushDateScale(getDate(stock[50])) },
+    end: { x: brushDateScale(getDate(stock[100])) },
+  };
+
   return (
     <div>
       <svg width={width} height={height}>
@@ -203,6 +208,7 @@ function BrushChart({
             handleSize={8}
             resizeTriggerAreas={['left', 'right', 'bottomRight']}
             brushDirection="horizontal"
+            initialBrushPosition={initialBrushPosition}
             onChange={onBrushChange}
             onClick={() => setFilteredStock(stock)}
             selectedBoxStyle={{

diff --git a/packages/vx-tooltip/test/Tooltip.test.tsx b/packages/vx-tooltip/test/Tooltip.test.tsx
@@ -19,7 +19,7 @@ describe('<Tooltip />', () => {
     const wrapper = shallow(<Tooltip unstyled>Hello</Tooltip>);
     const styles = wrapper.props().style;
     Object.keys(defaultStyles).forEach(key => {
-      expect(styles[key]).toBe(undefined);
+      expect(styles[key]).toBeUndefined();
     });
   });
 
@@ -34,7 +34,7 @@ describe('<Tooltip />', () => {
       boxShadow: '0 2px 3px rgba(133,133,133,0.5)',
       lineHeight: '2em',
     };
-    const wrapper = shallow(<Tooltip style={newStyles}></Tooltip>);
+    const wrapper = shallow(<Tooltip style={newStyles} />);
     const styles = wrapper.props().style;
     Object.entries(newStyles).forEach(([key, value]) => {
       expect(styles[key]).toBe(value);