From 8bd9c84fad9372b4533f232aac71ac0bad676664 Mon Sep 17 00:00:00 2001 From: xkxd Date: Wed, 1 Aug 2018 14:23:07 +0200 Subject: [PATCH 1/3] Make label conditionally optional, change box-sizing of field, change autoresize logic --- src/components/Input/README.md | 11 +++- .../Input/__snapshots__/input.spec.js.snap | 56 +++++++++++++++++++ src/components/Input/index.js | 29 ++++++++-- src/components/Input/input.spec.js | 7 +++ src/components/Input/styles.scss | 3 +- 5 files changed, 97 insertions(+), 9 deletions(-) diff --git a/src/components/Input/README.md b/src/components/Input/README.md index 6707f7a..b376924 100644 --- a/src/components/Input/README.md +++ b/src/components/Input/README.md @@ -24,6 +24,11 @@ Readonly input: ``` +Input with placeholder: +```js + +``` + A non-empty input with the error state: ```js @@ -31,12 +36,12 @@ A non-empty input with the error state: An non-empty textarea: ```js - + ``` An resizeable textarea: ```js - + ``` An autoresizeable textarea: @@ -46,5 +51,5 @@ An autoresizeable textarea: An autoresizeable textarea with custom initial number of rows: ```js - + ``` diff --git a/src/components/Input/__snapshots__/input.spec.js.snap b/src/components/Input/__snapshots__/input.spec.js.snap index adc8fdc..eeee4ef 100644 --- a/src/components/Input/__snapshots__/input.spec.js.snap +++ b/src/components/Input/__snapshots__/input.spec.js.snap @@ -17,6 +17,7 @@ exports[`Input autoresizing works correctly 1`] = ` onKeyDown={[Function]} onKeyPress={[Function]} onKeyUp={[Function]} + placeholder={null} readonly={false} resize="auto" rows={2} @@ -40,6 +41,7 @@ exports[`Input autoresizing works correctly 1`] = ` onKeyDown={[Function]} onKeyPress={[Function]} onKeyUp={[Function]} + placeholder={null} readOnly={false} rows={2} tabIndex={0} @@ -74,6 +76,7 @@ exports[`Input autoresizing works correctly 2`] = ` onKeyDown={[Function]} onKeyPress={[Function]} onKeyUp={[Function]} + placeholder={null} readonly={false} resize="auto" rows={2} @@ -97,6 +100,7 @@ exports[`Input autoresizing works correctly 2`] = ` onKeyDown={[Function]} onKeyPress={[Function]} onKeyUp={[Function]} + placeholder={null} readOnly={false} rows={2} style={ @@ -136,6 +140,7 @@ exports[`Input lifecycle works 1`] = ` onKeyDown={[Function]} onKeyPress={[Function]} onKeyUp={[Function]} + placeholder={null} readonly={false} resize={false} rows={2} @@ -158,6 +163,7 @@ exports[`Input lifecycle works 1`] = ` onKeyDown={[Function]} onKeyPress={[Function]} onKeyUp={[Function]} + placeholder={null} readOnly={false} tabIndex={0} type="text" @@ -190,6 +196,7 @@ exports[`Input lifecycle works 2`] = ` onKeyDown={[Function]} onKeyPress={[Function]} onKeyUp={[Function]} + placeholder={null} readonly={false} resize={false} rows={2} @@ -212,6 +219,7 @@ exports[`Input lifecycle works 2`] = ` onKeyDown={[Function]} onKeyPress={[Function]} onKeyUp={[Function]} + placeholder={null} readOnly={false} tabIndex={0} type="text" @@ -244,6 +252,7 @@ exports[`Input lifecycle works 3`] = ` onKeyDown={[Function]} onKeyPress={[Function]} onKeyUp={[Function]} + placeholder={null} readonly={false} resize={false} rows={2} @@ -266,6 +275,7 @@ exports[`Input lifecycle works 3`] = ` onKeyDown={[Function]} onKeyPress={[Function]} onKeyUp={[Function]} + placeholder={null} readOnly={false} tabIndex={0} type="text" @@ -296,6 +306,7 @@ exports[`Input renders correctly all the standard params 1`] = ` onKeyDown={[Function]} onKeyPress={[Function]} onKeyUp={[Function]} + placeholder={null} readOnly={false} tabIndex={0} type="text" @@ -325,6 +336,7 @@ exports[`Input renders correctly disabled 1`] = ` onKeyDown={[Function]} onKeyPress={[Function]} onKeyUp={[Function]} + placeholder={null} readOnly={false} tabIndex={0} type="text" @@ -354,6 +366,7 @@ exports[`Input renders correctly readonly 1`] = ` onKeyDown={[Function]} onKeyPress={[Function]} onKeyUp={[Function]} + placeholder={null} readOnly={true} tabIndex={0} type="text" @@ -368,6 +381,36 @@ exports[`Input renders correctly readonly 1`] = ` `; +exports[`Input renders correctly with a placeholder 1`] = ` +
+ + +
+`; + exports[`Input renders correctly with an error status and a hint 1`] = `
{ - this.setState({dynamicTextareaHeight: `${this.input.scrollHeight - BOTTOM_PADDING}px`}); + this.setState({dynamicTextareaHeight: `${this.input.scrollHeight + BOTTOM_BORDER_WIDTH}px`}); }); // to prevent scroll jumping @@ -278,11 +279,27 @@ Input.propTypes = { /** * Label that we want to display. */ - label: PropTypes.string.isRequired, + label: function(props, propName) { + if (props['placeholder'] && props[propName]) { + return new Error(`Prop ${propName} is not used when placeholder is set`); + } + + if (!props['placeholder'] && !props[propName]) { + return new Error(`Prop ${propName} is required when placeholder is not set`); + } + + if (typeof props[propName] !== 'string') { + return new Error(`Prop ${propName} is not a string`); + } + }, /** * Hint to display */ hint: PropTypes.string, + /** + * Placeholder to display + */ + placeholder: PropTypes.string, /** * Status */ @@ -347,7 +364,9 @@ Input.defaultProps = { autoFocus: false, className: '', disabled: false, + label: '', hint: null, + placeholder: null, hintClassName: '', id: null, inputClassName: '', diff --git a/src/components/Input/input.spec.js b/src/components/Input/input.spec.js index cf807bd..cacbb7d 100644 --- a/src/components/Input/input.spec.js +++ b/src/components/Input/input.spec.js @@ -66,6 +66,13 @@ test('Input renders correctly with an error status and a hint', () => { expect(component.toJSON()).toMatchSnapshot(); }); +test('Input renders correctly with a placeholder', () => { + const component = renderer.create( + , + ); + expect(component.toJSON()).toMatchSnapshot(); +}); + test('Input renders correctly with custom class names', () => { const component = renderer.create( Date: Wed, 1 Aug 2018 14:37:15 +0200 Subject: [PATCH 2/3] Fix tests and linting --- src/components/Input/__snapshots__/input.spec.js.snap | 2 +- src/components/Input/index.js | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/components/Input/__snapshots__/input.spec.js.snap b/src/components/Input/__snapshots__/input.spec.js.snap index eeee4ef..27e3fc9 100644 --- a/src/components/Input/__snapshots__/input.spec.js.snap +++ b/src/components/Input/__snapshots__/input.spec.js.snap @@ -105,7 +105,7 @@ exports[`Input autoresizing works correctly 2`] = ` rows={2} style={ Object { - "height": "98px", + "height": "101px", } } tabIndex={0} diff --git a/src/components/Input/index.js b/src/components/Input/index.js index 3ca3fb8..ab545d1 100644 --- a/src/components/Input/index.js +++ b/src/components/Input/index.js @@ -279,18 +279,20 @@ Input.propTypes = { /** * Label that we want to display. */ - label: function(props, propName) { - if (props['placeholder'] && props[propName]) { + label: (props, propName) => { + if (props.placeholder && props[propName]) { return new Error(`Prop ${propName} is not used when placeholder is set`); } - if (!props['placeholder'] && !props[propName]) { + if (!props.placeholder && !props[propName]) { return new Error(`Prop ${propName} is required when placeholder is not set`); } if (typeof props[propName] !== 'string') { return new Error(`Prop ${propName} is not a string`); } + + return null; }, /** * Hint to display From 50241bd00df2035046df8fb5c16ab9247453c957 Mon Sep 17 00:00:00 2001 From: xkxd Date: Wed, 1 Aug 2018 15:22:21 +0200 Subject: [PATCH 3/3] Add tests for proptypes --- src/components/Input/input.spec.js | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/components/Input/input.spec.js b/src/components/Input/input.spec.js index cacbb7d..e758c4b 100644 --- a/src/components/Input/input.spec.js +++ b/src/components/Input/input.spec.js @@ -11,6 +11,16 @@ jest.mock('lodash.uniqueid'); uniqueId.mockImplementation(() => 'foo'); /* eslint-disable no-console */ +function testPropTypeError(Component, props, expectedError) { + const stub = sinon.stub(console, 'error'); + + renderer.create(); + + expect(stub.calledOnce).toEqual(true); + expect(stub.calledWithMatch(sinon.match(new RegExp(expectedError)))).toEqual(true); + + console.error.restore(); +} test('Input renders correctly with default values', () => { const component = renderer.create( @@ -73,6 +83,22 @@ test('Input renders correctly with a placeholder', () => { expect(component.toJSON()).toMatchSnapshot(); }); +test('Input throws error when rendered with a placeholder and label', () => { + testPropTypeError( + Input, + {label: 'Label', placeholder: 'Placeholder'}, + 'Prop label is not used when placeholder is set' + ); +}); + +test('Input throws error when rendered without a placeholder or label', () => { + testPropTypeError(Input, {}, 'Prop label is required when placeholder is not set'); +}); + +test('Input throws error when rendered with invalid label', () => { + testPropTypeError(Input, {label: 100}, 'Prop label is not a string'); +}); + test('Input renders correctly with custom class names', () => { const component = renderer.create(