Merge pull request #790 from kristw/kristw--text

minor fix to <Text> and re-enable unit tests.
airbnb · Aug 27, 2020 · ea2bdec · ea2bdec
2 parents b860240 + f20f81e
commit ea2bdec
Show file tree

Hide file tree

Showing 5 changed files with 121 additions and 57 deletions.
diff --git a/packages/vx-text/package.json b/packages/vx-text/package.json
@@ -29,10 +29,10 @@
   "homepage": "https://github.com/hshoff/vx#readme",
   "dependencies": {
     "@types/classnames": "^2.2.9",
-    "@types/lodash": "^4.14.146",
+    "@types/lodash": "^4.14.160",
     "@types/react": "*",
     "classnames": "^2.2.5",
-    "lodash": "^4.17.15",
+    "lodash": "^4.17.20",
     "prop-types": "^15.7.2",
     "reduce-css-calc": "^1.3.0"
   },

diff --git a/packages/vx-text/src/Text.tsx b/packages/vx-text/src/Text.tsx
@@ -8,6 +8,15 @@ function isNumber(val: unknown): val is number {
   return typeof val === 'number';
 }
 
+function isValidXOrY(xOrY: string | number | undefined) {
+  return (
+    // number that is not NaN or Infinity
+    (typeof xOrY === 'number' && Number.isFinite(xOrY)) ||
+    // for percentage
+    typeof xOrY === 'string'
+  );
+}
+
 interface WordWithWidth {
   word: string;
   width: number;
@@ -24,7 +33,7 @@ type SVGTextProps = React.SVGAttributes<SVGTextElement>;
 type OwnProps = {
   /** className to apply to the SVGText element. */
   className?: string;
-  /** Whether to scale the fontSize to accomodate the specified width.  */
+  /** Whether to scale the fontSize to accommodate the specified width.  */
   scaleToFit?: boolean;
   /** Rotational angle of the text. */
   angle?: number;
@@ -176,6 +185,11 @@ class Text extends React.Component<TextProps, TextState> {
     const { wordsByLines } = this.state;
     const { x, y } = textProps;
 
+    // Cannot render <text> if x or y is invalid
+    if (!isValidXOrY(x) || !isValidXOrY(y)) {
+      return <svg ref={innerRef} x={dx} y={dy} fontSize={textProps.fontSize} style={SVG_STYLE} />;
+    }
+
     let startDy: string | undefined;
     if (verticalAnchor === 'start') {
       startDy = reduceCSSCalc(`calc(${capHeight})`);
@@ -187,7 +201,6 @@ class Text extends React.Component<TextProps, TextState> {
       startDy = reduceCSSCalc(`calc(${wordsByLines.length - 1} * -${lineHeight})`);
     }
 
-    let transform: string | undefined;
     const transforms = [];
     if (isNumber(x) && isNumber(y) && isNumber(width) && scaleToFit && wordsByLines.length > 0) {
       const lineWidth = wordsByLines[0].width || 1;
@@ -200,9 +213,8 @@ class Text extends React.Component<TextProps, TextState> {
     if (angle) {
       transforms.push(`rotate(${angle}, ${x}, ${y})`);
     }
-    if (transforms.length > 0) {
-      transform = transforms.join(' ');
-    }
+
+    const transform = transforms.length > 0 ? transforms.join(' ') : undefined;
 
     return (
       <svg ref={innerRef} x={dx} y={dy} fontSize={textProps.fontSize} style={SVG_STYLE}>

diff --git a/packages/vx-text/test/Text.test.tsx b/packages/vx-text/test/Text.test.tsx
@@ -1,6 +1,7 @@
 import React from 'react';
-import { shallow } from 'enzyme';
+import { shallow, mount } from 'enzyme';
 import { Text, getStringWidth } from '../src';
+import { addMock, removeMock } from './svgMock';
 
 describe('getStringWidth()', () => {
   it('should be defined', () => {
@@ -11,6 +12,9 @@ describe('getStringWidth()', () => {
 // TODO: Fix tests (jsdom does not support getComputedTextLength() or getBoundingClientRect()).  Maybe use puppeteer
 
 describe('<Text />', () => {
+  beforeEach(addMock);
+  afterEach(removeMock);
+
   it('should be defined', () => {
     expect(Text).toBeDefined();
   });
@@ -20,68 +24,81 @@ describe('<Text />', () => {
     expect(() => shallow(<Text>Hi</Text>)).not.toThrow();
   });
 
-  // it('Does not wrap long text if enough width', () => {
-  //   const wrapper = shallow(
-  //     <Text width={300} style={{ fontFamily: 'Courier' }}>This is really long text</Text>
-  //   );
+  it('Does not wrap long text if enough width', () => {
+    const wrapper = shallow<Text>(
+      <Text width={300} style={{ fontFamily: 'Courier' }}>
+        This is really long text
+      </Text>,
+    );
 
-  //   expect(wrapper.instance().state.wordsByLines.length).toEqual(1);
-  // });
+    expect(wrapper.instance().state.wordsByLines).toHaveLength(1);
+  });
 
-  // it('Wraps long text if not enough width', () => {
-  //   const wrapper = shallow(
-  //     <Text width={200} style={{ fontFamily: 'Courier' }}>This is really long text</Text>
-  //   );
+  it('Wraps text if not enough width', () => {
+    const wrapper = shallow<Text>(
+      <Text width={200} style={{ fontFamily: 'Courier' }}>
+        This is really long text
+      </Text>,
+    );
 
-  //   expect(wrapper.instance().state.wordsByLines.length).toEqual(2);
-  // });
+    expect(wrapper.instance().state.wordsByLines).toHaveLength(2);
+  });
 
-  // it('Wraps long text if styled but would have had enough room', () => {
-  //   const wrapper = shallow(
-  //     <Text width={300} style={{ fontSize: '2em', fontFamily: 'Courier' }}>This is really long text</Text>
-  //   );
+  it('Does not wrap text if there is enough width', () => {
+    const wrapper = shallow<Text>(
+      <Text width={300} style={{ fontSize: '2em', fontFamily: 'Courier' }}>
+        This is really long text
+      </Text>,
+    );
 
-  //   expect(wrapper.instance().state.wordsByLines.length).toEqual(2);
-  // });
+    expect(wrapper.instance().state.wordsByLines).toHaveLength(1);
+  });
 
-  // it('Does not perform word length calculation if width or scaleToFit props not set', () => {
-  //   const wrapper = shallow(
-  //     <Text>This is really long text</Text>
-  //   );
+  it('Does not perform word length calculation if width or scaleToFit props not set', () => {
+    const wrapper = shallow<Text>(<Text>This is really long text</Text>);
 
-  //   expect(wrapper.instance().state.wordsByLines.length).toEqual(1);
-  //   expect(wrapper.instance().state.wordsByLines[0].width).toEqual(undefined);
-  // });
+    expect(wrapper.instance().state.wordsByLines).toHaveLength(1);
+    expect(wrapper.instance().state.wordsByLines[0].width).toBeUndefined();
+  });
 
-  // it('Render 0 success when specify the width', () => {
-  //   const wrapper = render(
-  //     <Text x={0} y={0} width={30}>{0}</Text>
-  //   );
+  it('Render 0 success when specify the width', () => {
+    const wrapper = mount(
+      <Text x={0} y={0} width={30}>
+        0
+      </Text>,
+    );
 
-  //   expect(wrapper.text()).toContain('0');
-  // });
+    console.log('wrapper', wrapper.text());
+    expect(wrapper.text()).toContain('0');
+  });
 
-  // it('Render 0 success when not specify the width', () => {
-  //   const wrapper = render(
-  //     <Text x={0} y={0}>{0}</Text>
-  //   );
+  it('Render 0 success when not specify the width', () => {
+    const wrapper = mount(
+      <Text x={0} y={0}>
+        0
+      </Text>,
+    );
 
-  //   expect(wrapper.text()).toContain('0');
-  // });
+    expect(wrapper.text()).toContain('0');
+  });
 
-  // it('Render text when x or y is a percentage', () => {
-  //   const wrapper = render(
-  //     <Text x="50%" y="50%">anything</Text>
-  //   );
+  it('Render text when x or y is a percentage', () => {
+    const wrapper = mount(
+      <Text x="50%" y="50%">
+        anything
+      </Text>,
+    );
 
-  //   expect(wrapper.text()).toContain('anything');
-  // });
+    expect(wrapper.text()).toContain('anything');
+  });
 
-  // it("Don't Render text when x or y is NaN ", () => {
-  //   const wrapperNan = render(
-  //     <Text x={NaN} y={10}>anything</Text>
-  //   );
+  it("Don't Render text when x or y is NaN", () => {
+    const wrapperNan = mount(
+      <Text x={NaN} y={10}>
+        anything
+      </Text>,
+    );
 
-  //   expect(wrapperNan.text()).not.toContain('anything');
-  // });
+    expect(wrapperNan.text()).not.toContain('anything');
+  });
 });
diff --git a/packages/vx-text/test/svgMock.ts b/packages/vx-text/test/svgMock.ts
@@ -0,0 +1,25 @@
+// @ts-ignore
+let originalFn: typeof SVGElement.prototype.getComputedTextLength;
+
+/**
+ * JSDom does not implement getComputedTextLength()
+ * so this function add mock implementation for testing.
+ */
+export function addMock() {
+  // @ts-ignore
+  originalFn = SVGElement.prototype.getComputedTextLength;
+
+  // @ts-ignore
+  SVGElement.prototype.getComputedTextLength = function getComputedTextLength() {
+    // Make every character 10px wide
+    return (this.textContent?.length ?? 0) * 10;
+  };
+}
+
+/**
+ * Remove mock from addMock()
+ */
+export function removeMock() {
+  // @ts-ignore
+  SVGElement.prototype.getComputedTextLength = originalFn;
+}
diff --git a/yarn.lock b/yarn.lock
@@ -2841,6 +2841,11 @@
   resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.154.tgz#069e3c703fdb264e67be9e03b20a640bc0198ecc"
   integrity sha512-VoDZIJmg3P8vPEnTldLvgA+q7RkIbVkbYX4k0cAVFzGAOQwUehVgRHgIr2/wepwivDst/rVRqaiBSjCXRnoWwQ==
 
+"@types/lodash@^4.14.160":
+  version "4.14.160"
+  resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.160.tgz#2f1bba6500bc3cb9a732c6d66a083378fb0b0b29"
+  integrity sha512-aP03BShJoO+WVndoVj/WNcB/YBPt+CIU1mvaao2GRAHy2yg4pT/XS4XnVHEQBjPJGycWf/9seKEO9vopTJGkvA==
+
 "@types/micromatch@^4.0.1":
   version "4.0.1"
   resolved "https://registry.yarnpkg.com/@types/micromatch/-/micromatch-4.0.1.tgz#9381449dd659fc3823fd2a4190ceacc985083bc7"
@@ -8909,6 +8914,11 @@ lodash@^4.15.0, lodash@^4.17.10, lodash@^4.17.11, lodash@^4.17.12, lodash@^4.17.
   resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.15.tgz#b447f6670a0455bbfeedd11392eff330ea097548"
   integrity sha512-8xOcRHvCjnocdS5cpwXQXVzmmh5e5+saE2QGoeQmbKmRS6J3VQppPOIt0MnmE+4xlZoumy0GPG0D0MVIQbNA1A==
 
+lodash@^4.17.20:
+  version "4.17.20"
+  resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.20.tgz#b44a9b6297bcb698f1c51a3545a2b3b368d59c52"
+  integrity sha512-PlhdFcillOINfeV7Ni6oF1TAEayyZBoZ8bcshTHqOYJYlrqzRK5hagpagky5o4HfCzzd1TRkXPMFq6cKk9rGmA==
+
 log-driver@^1.2.7:
   version "1.2.7"
   resolved "https://registry.yarnpkg.com/log-driver/-/log-driver-1.2.7.tgz#63b95021f0702fedfa2c9bb0a24e7797d71871d8"