From c7225f07492831d21a7a41420408b1706fa536c9 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Wed, 26 Aug 2020 13:35:11 -0700 Subject: [PATCH] fix: text rendering with invalid x or y --- packages/vx-text/package.json | 5 +- packages/vx-text/src/Text.tsx | 22 +++- packages/vx-text/src/util/getStringWidth.ts | 2 +- packages/vx-text/test/Text.test.tsx | 117 +++++++++++--------- packages/vx-text/test/svgMock.ts | 25 +++++ yarn.lock | 12 ++ 6 files changed, 124 insertions(+), 59 deletions(-) create mode 100644 packages/vx-text/test/svgMock.ts diff --git a/packages/vx-text/package.json b/packages/vx-text/package.json index 2a42132ee..5bd7fa92f 100644 --- a/packages/vx-text/package.json +++ b/packages/vx-text/package.json @@ -29,11 +29,10 @@ "homepage": "https://github.com/hshoff/vx#readme", "dependencies": { "@types/classnames": "^2.2.9", - "@types/lodash": "^4.14.146", + "@types/lodash.memoize": "^4.1.6", "@types/react": "*", "classnames": "^2.2.5", - "lodash": "^4.17.15", - "prop-types": "^15.7.2", + "lodash.memoize": "^4.1.2", "reduce-css-calc": "^1.3.0" }, "peerDependencies": { diff --git a/packages/vx-text/src/Text.tsx b/packages/vx-text/src/Text.tsx index 50614f736..6022b3fff 100644 --- 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; 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 { const { wordsByLines } = this.state; const { x, y } = textProps; + // Cannot render if x or y is invalid + if (!isValidXOrY(x) || !isValidXOrY(y)) { + return ; + } + let startDy: string | undefined; if (verticalAnchor === 'start') { startDy = reduceCSSCalc(`calc(${capHeight})`); @@ -187,7 +201,6 @@ class Text extends React.Component { 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 { if (angle) { transforms.push(`rotate(${angle}, ${x}, ${y})`); } - if (transforms.length > 0) { - transform = transforms.join(' '); - } + + const transform = transforms.length > 0 ? transforms.join(' ') : undefined; return ( diff --git a/packages/vx-text/src/util/getStringWidth.ts b/packages/vx-text/src/util/getStringWidth.ts index 0fc97464b..7b6ab104b 100644 --- a/packages/vx-text/src/util/getStringWidth.ts +++ b/packages/vx-text/src/util/getStringWidth.ts @@ -1,4 +1,4 @@ -import memoize from 'lodash/memoize'; +import memoize from 'lodash.memoize'; const MEASUREMENT_ELEMENT_ID = '__react_svg_text_measurement_id'; diff --git a/packages/vx-text/test/Text.test.tsx b/packages/vx-text/test/Text.test.tsx index db3ad45a3..412df2a5f 100644 --- 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('', () => { + beforeEach(addMock); + afterEach(removeMock); + it('should be defined', () => { expect(Text).toBeDefined(); }); @@ -20,68 +24,81 @@ describe('', () => { expect(() => shallow(Hi)).not.toThrow(); }); - // it('Does not wrap long text if enough width', () => { - // const wrapper = shallow( - // This is really long text - // ); + it('Does not wrap long text if enough width', () => { + const wrapper = shallow( + + This is really long 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( - // This is really long text - // ); + it('Wraps text if not enough width', () => { + const wrapper = shallow( + + This is really long 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( - // This is really long text - // ); + it('Does not wrap text if there is enough width', () => { + const wrapper = shallow( + + This is really long 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( - // This is really long text - // ); + it('Does not perform word length calculation if width or scaleToFit props not set', () => { + const wrapper = shallow(This is really long 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( - // {0} - // ); + it('Render 0 success when specify the width', () => { + const wrapper = mount( + + 0 + , + ); - // 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( - // {0} - // ); + it('Render 0 success when not specify the width', () => { + const wrapper = mount( + + 0 + , + ); - // expect(wrapper.text()).toContain('0'); - // }); + expect(wrapper.text()).toContain('0'); + }); - // it('Render text when x or y is a percentage', () => { - // const wrapper = render( - // anything - // ); + it('Render text when x or y is a percentage', () => { + const wrapper = mount( + + anything + , + ); - // expect(wrapper.text()).toContain('anything'); - // }); + expect(wrapper.text()).toContain('anything'); + }); - // it("Don't Render text when x or y is NaN ", () => { - // const wrapperNan = render( - // anything - // ); + it("Don't Render text when x or y is NaN", () => { + const wrapperNan = mount( + + anything + , + ); - // 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 new file mode 100644 index 000000000..0dc37e169 --- /dev/null +++ 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 index 20959cc91..e0ddd8325 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2836,6 +2836,18 @@ resolved "https://registry.yarnpkg.com/@types/json-schema/-/json-schema-7.0.4.tgz#38fd73ddfd9b55abb1e1b2ed578cb55bd7b7d339" integrity sha512-8+KAKzEvSUdeo+kmqnKrqgeE+LcA0tjYWFY7RPProVYwnqDjukzO+3b6dLD56rYX5TdWejnEOLJYOIeh4CXKuA== +"@types/lodash.memoize@^4.1.6": + version "4.1.6" + resolved "https://registry.yarnpkg.com/@types/lodash.memoize/-/lodash.memoize-4.1.6.tgz#3221f981790a415cab1a239f25c17efd8b604c23" + integrity sha512-mYxjKiKzRadRJVClLKxS4wb3Iy9kzwJ1CkbyKiadVxejnswnRByyofmPMscFKscmYpl36BEEhCMPuWhA1R/1ZQ== + dependencies: + "@types/lodash" "*" + +"@types/lodash@*": + 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/lodash@^4.14.146": version "4.14.154" resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.154.tgz#069e3c703fdb264e67be9e03b20a640bc0198ecc"