minor fix to <Text> and re-enable unit tests. #790
Conversation
packages/vx-text/package.json
Outdated
| "@types/react": "*", | ||
| "classnames": "^2.2.5", | ||
| "lodash": "^4.17.15", | ||
| "prop-types": "^15.7.2", | ||
| "lodash.memoize": "^4.1.2", |
There was a problem hiding this comment.
I've gone through and done this before but had forgot this was intentional, see #66
There was a problem hiding this comment.
oh ok sure. will revert.
| "@types/react": "*", | ||
| "classnames": "^2.2.5", | ||
| "lodash": "^4.17.15", | ||
| "prop-types": "^15.7.2", |
There was a problem hiding this comment.
we actually need this because we have a babel plugin that adds proptypes based on the types defined for a component.
this is useful for consumers I think (proptypes complain in dev / in the console), but @hshoff also was surprised by this recently so maybe we could consider removing it througout.
| @@ -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} />; | |||
There was a problem hiding this comment.
do you think we should return null or an svg?
There was a problem hiding this comment.
I was thinking about return null but think if at least having the svg may keep the layout from changing if consumer depends on having some element there.
| * JSDom does not implement getComputedTextLength() | ||
| * so this function add mock implementation for testing. | ||
| */ | ||
| export function addMock() { |
🐛 Bug Fix
NaNor invalid values are passed asxory🏠 Internal
prop-typesfrom dependency. (no longer used)lodash.memoizein dependency ofvx/textwhich is smaller than entirelodash.