fix(shape, xychart): default LinePath strokeLinecap to round (#1105)

* fix(shape): default LinePath strokeLinecap to round * fix(xychart): default Line/AreaSeries to strokeLinecap='round' for missing data
airbnb · Mar 12, 2021 · 5c2f802 · 5c2f802
1 parent dcc1ccb
commit 5c2f802
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 7 deletions.
diff --git a/packages/visx-shape/src/shapes/LinePath.tsx b/packages/visx-shape/src/shapes/LinePath.tsx
@@ -38,6 +38,9 @@ export default function LinePath<Datum>({
       className={cx('visx-linepath', className)}
       d={path(data) || ''}
       fill={fill}
+      // without this a datum surrounded by nulls will not be visible
+      // https://github.com/d3/d3-shape#line_defined
+      strokeLinecap="round"
       {...restProps}
     />
   );

diff --git a/packages/visx-shape/test/LinePath.test.tsx b/packages/visx-shape/test/LinePath.test.tsx
@@ -23,33 +23,37 @@ const LinePathChildren = ({ children, ...restProps }: Partial<LinePathProps<Datu
   shallow(<LinePath {...restProps}>{children}</LinePath>);
 
 describe('<LinePath />', () => {
-  test('it should be defined', () => {
+  it('should be defined', () => {
     expect(LinePath).toBeDefined();
   });
 
-  test('it should have the .visx-linepath class', () => {
+  it('should have the .visx-linepath class', () => {
     expect(LinePathWrapper(linePathProps).prop('className')).toBe('visx-linepath');
   });
 
-  test('it should contain paths', () => {
+  it('should default to strokeLinecap="round" for superior missing data rendering', () => {
+    expect(LinePathWrapper(linePathProps).prop('strokeLinecap')).toBe('round');
+  });
+
+  it('should contain paths', () => {
     expect(LinePathWrapper(linePathProps).find('path').length).toBeGreaterThan(0);
   });
 
-  test('it should take a children as function prop', () => {
+  it('should take a children as function prop', () => {
     const fn = jest.fn();
     LinePathChildren({ children: fn });
     expect(fn).toHaveBeenCalled();
   });
 
-  test('it should call children function with { path }', () => {
+  it('should call children function with { path }', () => {
     const fn = jest.fn();
     LinePathChildren({ children: fn });
     const args = fn.mock.calls[0][0];
     const keys = Object.keys(args);
-    expect(keys.includes('path')).toEqual(true);
+    expect(keys).toContain('path');
   });
 
-  test('it should expose its ref via an innerRef prop', () => {
+  it('should expose its ref via an innerRef prop', () => {
     // eslint-disable-next-line jest/no-test-return-statement
     return new Promise(done => {
       const refCallback = (ref: SVGPathElement) => {

diff --git a/packages/visx-xychart/src/components/series/private/BaseAreaSeries.tsx b/packages/visx-xychart/src/components/series/private/BaseAreaSeries.tsx
@@ -129,6 +129,7 @@ function BaseAreaSeries<XScale extends AxisScale, YScale extends AxisScale, Datu
             className="visx-area"
             stroke="transparent"
             fill={color}
+            strokeLinecap="round" // without this a datum surrounded by nulls will not be visible
             {...areaProps}
             d={path(data) || ''}
             {...eventEmitters}
@@ -150,6 +151,7 @@ function BaseAreaSeries<XScale extends AxisScale, YScale extends AxisScale, Datu
               stroke={color}
               strokeWidth={2}
               pointerEvents="none"
+              strokeLinecap="round" // without this a datum surrounded by nulls will not be visible
               {...lineProps}
               d={path(data) || ''}
             />

diff --git a/packages/visx-xychart/src/components/series/private/BaseLineSeries.tsx b/packages/visx-xychart/src/components/series/private/BaseLineSeries.tsx
@@ -88,6 +88,7 @@ function BaseLineSeries<XScale extends AxisScale, YScale extends AxisScale, Datu
             stroke={color}
             strokeWidth={2}
             fill="transparent"
+            strokeLinecap="round" // without this a datum surrounded by nulls will not be visible
             {...lineProps}
             d={path(data) || ''}
             {...eventEmitters}

diff --git a/packages/visx-xychart/test/components/AreaSeries.test.tsx b/packages/visx-xychart/test/components/AreaSeries.test.tsx
@@ -26,6 +26,17 @@ describe('<AreaSeries />', () => {
     expect(wrapper.find(Area)).toHaveLength(1);
   });
 
+  it('should set strokeLinecap="round" to make datum surrounded by nulls visible', () => {
+    const wrapper = mount(
+      <DataContext.Provider value={getDataContext(series)}>
+        <svg>
+          <AreaSeries dataKey={series.key} renderLine={false} {...series} />
+        </svg>
+      </DataContext.Provider>,
+    );
+    expect(wrapper.find('path').prop('strokeLinecap')).toBe('round');
+  });
+
   it('should use x/y0Accessors an Area', () => {
     const y0Accessor = jest.fn(() => 3);
     const wrapper = mount(

diff --git a/packages/visx-xychart/test/components/LineSeries.test.tsx b/packages/visx-xychart/test/components/LineSeries.test.tsx
@@ -26,6 +26,17 @@ describe('<LineSeries />', () => {
     expect(wrapper.find(LinePath)).toHaveLength(1);
   });
 
+  it('should set strokeLinecap="round" to make datum surrounded by nulls visible', () => {
+    const wrapper = mount(
+      <DataContext.Provider value={getDataContext(series)}>
+        <svg>
+          <LineSeries dataKey={series.key} {...series} />
+        </svg>
+      </DataContext.Provider>,
+    );
+    expect(wrapper.find('path').prop('strokeLinecap')).toBe('round');
+  });
+
   it('should render Glyphs if focus/blur handlers are set', () => {
     const wrapper = mount(
       <DataContext.Provider value={getDataContext(series)}>