From 6ced8e0fab188dab392894e225e57fb5454975a8 Mon Sep 17 00:00:00 2001 From: Andre Mas Date: Mon, 14 Aug 2017 14:09:10 -0400 Subject: [PATCH 01/10] fixes issue #84, for pasting with delimiters --- lib/ReactTags.js | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/lib/ReactTags.js b/lib/ReactTags.js index ce40f3b..34be2b1 100644 --- a/lib/ReactTags.js +++ b/lib/ReactTags.js @@ -56,6 +56,42 @@ class ReactTags extends React.Component { this.setState({ query }) } + handlePaste (e) { + // allow over-ride, if there is a need + if (this.props.onPaste) { + return this.props.onPaste(e); + } + + const { delimiters, delimiterChars } = this.props + + e.preventDefault() + + // get the text data from the clipboard + const data = e.clipboardData.getData('Text') + + if (data && delimiterChars.length > 0) { + // split the string based on the delimiterChars as a regex + const tags = data.split(new RegExp(delimiterChars.join('|'), 'g')); + for (let i=0; i < tags.length; i++) { + // the logic here is similar to handleKeyDown, but subtly different, + // due to the context of the operation + if (tags[i].length > 0) { + // look to see if the tag is already known + const matchIdx = this.suggestions.state.options.findIndex((suggestion) => { + return tags[i] === suggestion.name + }) + + // if already known add it, otherwise add it only if we allow new tags + if (matchIdx > -1) { + this.addTag(this.suggestions.state.options[matchIdx]) + } else if (this.props.allowNew) { + this.addTag({ name: tags[i] }) + } + } + } + } + } + handleKeyDown (e) { const { query, selectedIndex } = this.state const { delimiters, delimiterChars } = this.props @@ -166,14 +202,16 @@ class ReactTags extends React.Component { onBlur={this.handleBlur.bind(this)} onFocus={this.handleFocus.bind(this)} onChange={this.handleChange.bind(this)} - onKeyDown={this.handleKeyDown.bind(this)}> + onKeyDown={this.handleKeyDown.bind(this)} + onPaste={this.handlePaste.bind(this)} > { this.input = c }} listboxId={listboxId} autofocus={this.props.autofocus} autoresize={this.props.autoresize} expandable={expandable} - placeholder={this.props.placeholder} /> + placeholder={this.props.placeholder} + /> { this.suggestions = c }} listboxId={listboxId} From 27939c1d1b90765e43a45ecb0ecbe2f11099836f Mon Sep 17 00:00:00 2001 From: Andre Mas Date: Mon, 14 Aug 2017 14:18:52 -0400 Subject: [PATCH 02/10] removed trailing whitespace --- lib/ReactTags.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ReactTags.js b/lib/ReactTags.js index 34be2b1..c4723af 100644 --- a/lib/ReactTags.js +++ b/lib/ReactTags.js @@ -63,7 +63,7 @@ class ReactTags extends React.Component { } const { delimiters, delimiterChars } = this.props - + e.preventDefault() // get the text data from the clipboard @@ -73,7 +73,7 @@ class ReactTags extends React.Component { // split the string based on the delimiterChars as a regex const tags = data.split(new RegExp(delimiterChars.join('|'), 'g')); for (let i=0; i < tags.length; i++) { - // the logic here is similar to handleKeyDown, but subtly different, + // the logic here is similar to handleKeyDown, but subtly different, // due to the context of the operation if (tags[i].length > 0) { // look to see if the tag is already known @@ -88,7 +88,7 @@ class ReactTags extends React.Component { this.addTag({ name: tags[i] }) } } - } + } } } From d364403b02103964c3e1e4b2b55acd99656868ea Mon Sep 17 00:00:00 2001 From: Andre Mas Date: Mon, 14 Aug 2017 14:21:19 -0400 Subject: [PATCH 03/10] fixed issues causing linter to complain --- lib/ReactTags.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/ReactTags.js b/lib/ReactTags.js index c4723af..2650e36 100644 --- a/lib/ReactTags.js +++ b/lib/ReactTags.js @@ -59,10 +59,10 @@ class ReactTags extends React.Component { handlePaste (e) { // allow over-ride, if there is a need if (this.props.onPaste) { - return this.props.onPaste(e); + return this.props.onPaste(e) } - const { delimiters, delimiterChars } = this.props + const { delimiterChars } = this.props e.preventDefault() @@ -71,8 +71,8 @@ class ReactTags extends React.Component { if (data && delimiterChars.length > 0) { // split the string based on the delimiterChars as a regex - const tags = data.split(new RegExp(delimiterChars.join('|'), 'g')); - for (let i=0; i < tags.length; i++) { + const tags = data.split(new RegExp(delimiterChars.join('|'), 'g')) + for (let i = 0; i < tags.length; i++) { // the logic here is similar to handleKeyDown, but subtly different, // due to the context of the operation if (tags[i].length > 0) { From 5fae01c20adbabe3538497b58590cad9319cb3dc Mon Sep 17 00:00:00 2001 From: Andre Mas Date: Mon, 14 Aug 2017 17:18:27 -0400 Subject: [PATCH 04/10] Added test case for onPaste --- spec/ReactTags.spec.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/spec/ReactTags.spec.js b/spec/ReactTags.spec.js index 947443e..4b8cc76 100644 --- a/spec/ReactTags.spec.js +++ b/spec/ReactTags.spec.js @@ -64,6 +64,10 @@ function click (target) { TestUtils.Simulate.click(target) } +function paste (target, eventData) { + TestUtils.Simulate.paste(target, eventData) +} + describe('React Tags', () => { afterEach(() => { teardownInstance() @@ -371,6 +375,17 @@ describe('React Tags', () => { expect($$('.custom-tag').length).toEqual(2) }) + + it('can receive tags through paste, respecting delimiters', () => { + createInstance({ allowNew: true, delimiterChars: [',', ';'] }) + + paste($('input'), { clipboardData: { + types: ['text/plain', 'Text'], + getData: (type) => 'foo,bar;baz' + }}) + + sinon.assert.calledThrice(props.handleAddition) + }) }) describe('sizer', () => { From 89b68b391a5d1c8d6ba2c2328e8c65267dbcd699 Mon Sep 17 00:00:00 2001 From: Andre Mas Date: Mon, 14 Aug 2017 18:53:24 -0400 Subject: [PATCH 05/10] Changes following review --- lib/ReactTags.js | 14 ++++---- spec/ReactTags.spec.js | 72 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 78 insertions(+), 8 deletions(-) diff --git a/lib/ReactTags.js b/lib/ReactTags.js index 2650e36..90e3a71 100644 --- a/lib/ReactTags.js +++ b/lib/ReactTags.js @@ -58,8 +58,8 @@ class ReactTags extends React.Component { handlePaste (e) { // allow over-ride, if there is a need - if (this.props.onPaste) { - return this.props.onPaste(e) + if (this.props.handlePaste) { + return this.props.handlePaste(e) } const { delimiterChars } = this.props @@ -70,20 +70,21 @@ class ReactTags extends React.Component { const data = e.clipboardData.getData('Text') if (data && delimiterChars.length > 0) { - // split the string based on the delimiterChars as a regex - const tags = data.split(new RegExp(delimiterChars.join('|'), 'g')) + // split the string based on the delimiterChars as a regex, being sure + // to escape chars, to prevent them being treated as special characters + const tags = data.split(new RegExp('[\\' + delimiterChars.join('\\') + ']')) for (let i = 0; i < tags.length; i++) { // the logic here is similar to handleKeyDown, but subtly different, // due to the context of the operation if (tags[i].length > 0) { // look to see if the tag is already known - const matchIdx = this.suggestions.state.options.findIndex((suggestion) => { + const matchIdx = this.props.suggestions.findIndex((suggestion) => { return tags[i] === suggestion.name }) // if already known add it, otherwise add it only if we allow new tags if (matchIdx > -1) { - this.addTag(this.suggestions.state.options[matchIdx]) + this.addTag(this.props.suggestions[matchIdx]) } else if (this.props.allowNew) { this.addTag({ name: tags[i] }) } @@ -251,6 +252,7 @@ ReactTags.propTypes = { handleDelete: PropTypes.func.isRequired, handleAddition: PropTypes.func.isRequired, handleInputChange: PropTypes.func, + handlePaste: PropTypes.func, minQueryLength: PropTypes.number, maxSuggestionsLength: PropTypes.number, classNames: PropTypes.object, diff --git a/spec/ReactTags.spec.js b/spec/ReactTags.spec.js index 4b8cc76..94b9ce2 100644 --- a/spec/ReactTags.spec.js +++ b/spec/ReactTags.spec.js @@ -377,14 +377,63 @@ describe('React Tags', () => { }) it('can receive tags through paste, respecting delimiters', () => { - createInstance({ allowNew: true, delimiterChars: [',', ';'] }) + // The large range of delimiterChars in the test is to ensure + // they don't take on new meaning when used as part of a regex. + createInstance({ + allowNew: true, + delimiterChars: ['^', ',', ';', '.', '\\', '['], + suggestions: [{ id: 1, name: 'foo' }] + }) + + paste($('input'), { clipboardData: { + types: ['text/plain', 'Text'], + getData: (type) => 'foo,bar;baz^fam\\bam[moo' + }}) + + sinon.assert.callCount(props.handleAddition, 6) + }) + + it('can receive tags through paste, ignoring new tags', () => { + createInstance({ + allowNew: false, + delimiterChars: [','], + suggestions: [{ id: 1, name: 'foo' }] + }) + + paste($('input'), { clipboardData: { + types: ['text/plain', 'Text'], + getData: (type) => 'foo,bar' + }}) + + sinon.assert.callCount(props.handleAddition, 1) + }) + + it('accepts no paste, if there are not delimiterChars', () => { + createInstance({ + allowNew: true, + delimiterChars: [] + }) paste($('input'), { clipboardData: { types: ['text/plain', 'Text'], getData: (type) => 'foo,bar;baz' }}) - sinon.assert.calledThrice(props.handleAddition) + sinon.assert.callCount(props.handleAddition, 0) + }) + + it('adds no tags, if the pasted string is empty', () => { + createInstance({ + allowNew: true, + delimiterChars: [','] + }) + + paste($('input'), { clipboardData: { + types: ['text/plain', 'Text'], + getData: (type) => '' + }}) + + sinon.assert.callCount(props.handleAddition, 0) }) }) @@ -450,4 +499,23 @@ describe('React Tags', () => { expect(input.style.width).toBeFalsy() }) }) + + describe('event override', () => { + it('can receive tags through paste, respecting delimiters', () => { + createInstance({ + allowNew: true, + delimiterChars: [','], + handlePaste: (e) => { + e.preventDefault() + } + }) + + paste($('input'), { clipboardData: { + types: ['text/plain', 'Text'], + getData: (type) => 'foo,bar;baz' + }}) + + sinon.assert.callCount(props.handleAddition, 0) + }) + }) }) From 88a33b2ded6da7850f076f48d20e8a39c3def213 Mon Sep 17 00:00:00 2001 From: Andre Mas Date: Mon, 14 Aug 2017 19:16:25 -0400 Subject: [PATCH 06/10] Added missing changes, added extra tests --- lib/ReactTags.js | 18 ++++++++++++++++++ spec/ReactTags.spec.js | 8 ++++++++ 2 files changed, 26 insertions(+) diff --git a/lib/ReactTags.js b/lib/ReactTags.js index 90e3a71..1bd38ed 100644 --- a/lib/ReactTags.js +++ b/lib/ReactTags.js @@ -38,6 +38,24 @@ class ReactTags extends React.Component { selectedIndex: -1, classNames: Object.assign({}, CLASS_NAMES, this.props.classNames) } + + this.checkDelimiterChars(props.delimiterChars) + } + + /** + * Checks that each entry of delimiterChars is only a single + * character long. + * + * @param {Array} delimiterChars + */ + checkDelimiterChars (delimiterChars) { + if (delimiterChars && delimiterChars.length > 0) { + delimiterChars.forEach((delimiterChar) => { + if (delimiterChar.length > 1) { + throw new Error(`DelimiterChar entries should be single characters, invalid: ${delimiterChar}`) + } + }) + } } componentWillReceiveProps (newProps) { diff --git a/spec/ReactTags.spec.js b/spec/ReactTags.spec.js index 94b9ce2..43c33a1 100644 --- a/spec/ReactTags.spec.js +++ b/spec/ReactTags.spec.js @@ -376,6 +376,14 @@ describe('React Tags', () => { expect($$('.custom-tag').length).toEqual(2) }) + it('will throw an error if a delimiter is longer than one character', () => { + expect(() => createInstance({ + allowNew: true, + delimiterChars: [',', ',X'], + suggestions: [{ id: 1, name: 'foo' }] + })).toThrow() + }) + it('can receive tags through paste, respecting delimiters', () => { // The large range of delimiterChars in the test is to ensure // they don't take on new meaning when used as part of a regex. From 56b2d109454221b3545973a2e28e38fc68bec43e Mon Sep 17 00:00:00 2001 From: Andre Mas Date: Tue, 15 Aug 2017 11:55:49 -0400 Subject: [PATCH 07/10] Fixes following review Changes include - Not preventing multi-character delimiters - Indicating multi-chracter delimiters are only supported in paste operations, mainly because handleKeyDown will not deal with them - Changing the default delimiterChars to be consistent with the set of default delimiter keycodes --- README.md | 2 +- lib/ReactTags.js | 26 ++++++-------------------- spec/ReactTags.spec.js | 29 +++++++++++++++++++---------- 3 files changed, 26 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 8ff1389..bfaf064 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,7 @@ Array of integers matching keyboard event `keyCode` values. When a corresponding #### delimiterChars (optional) -Array of characters matching keyboard event `key` values. This is useful when needing to support a specific character irrespective of the keyboard layout. Note, that this list is separate from the one specified by the `delimiters` option, so you'll need to set the value there to `[]`, if you wish to disable those keys. Example usage: `delimiterChars={[',', ' ']}`. Default: `[]` +Array of characters matching keyboard event `key` values. This is useful when needing to support a specific character irrespective of the keyboard layout. Note, that this list is separate from the one specified by the `delimiters` option, so you'll need to set the value there to `[]`, if you wish to disable those keys. Note, that specifying delimiters longer than one character is only supported for paste operations. Example usage: `delimiterChars={[',', ' ']}`. Default: `['\t', '\r\n', '\r', '\n']` #### minQueryLength (optional) diff --git a/lib/ReactTags.js b/lib/ReactTags.js index 1bd38ed..e0ef103 100644 --- a/lib/ReactTags.js +++ b/lib/ReactTags.js @@ -38,24 +38,6 @@ class ReactTags extends React.Component { selectedIndex: -1, classNames: Object.assign({}, CLASS_NAMES, this.props.classNames) } - - this.checkDelimiterChars(props.delimiterChars) - } - - /** - * Checks that each entry of delimiterChars is only a single - * character long. - * - * @param {Array} delimiterChars - */ - checkDelimiterChars (delimiterChars) { - if (delimiterChars && delimiterChars.length > 0) { - delimiterChars.forEach((delimiterChar) => { - if (delimiterChar.length > 1) { - throw new Error(`DelimiterChar entries should be single characters, invalid: ${delimiterChar}`) - } - }) - } } componentWillReceiveProps (newProps) { @@ -64,6 +46,10 @@ class ReactTags extends React.Component { }) } + escapeForRegExp (query) { + return query.replace(/[-\\^$*+?.()|[\]{}]/g, '\\$&') + } + handleChange (e) { const query = e.target.value @@ -90,7 +76,7 @@ class ReactTags extends React.Component { if (data && delimiterChars.length > 0) { // split the string based on the delimiterChars as a regex, being sure // to escape chars, to prevent them being treated as special characters - const tags = data.split(new RegExp('[\\' + delimiterChars.join('\\') + ']')) + const tags = data.split(new RegExp('[' + this.escapeForRegExp(delimiterChars.join('')) + ']')) for (let i = 0; i < tags.length; i++) { // the logic here is similar to handleKeyDown, but subtly different, // due to the context of the operation @@ -251,7 +237,7 @@ ReactTags.defaultProps = { autofocus: true, autoresize: true, delimiters: [KEYS.TAB, KEYS.ENTER], - delimiterChars: [], + delimiterChars: ['\t', '\r\n', '\r', '\n'], minQueryLength: 2, maxSuggestionsLength: 6, allowNew: false, diff --git a/spec/ReactTags.spec.js b/spec/ReactTags.spec.js index 43c33a1..d612d44 100644 --- a/spec/ReactTags.spec.js +++ b/spec/ReactTags.spec.js @@ -376,29 +376,38 @@ describe('React Tags', () => { expect($$('.custom-tag').length).toEqual(2) }) - it('will throw an error if a delimiter is longer than one character', () => { - expect(() => createInstance({ - allowNew: true, - delimiterChars: [',', ',X'], - suggestions: [{ id: 1, name: 'foo' }] - })).toThrow() + it('can receive tags through paste, respecting default delimiter chars', () => { + // The large range of delimiterChars in the test is to ensure + // they don't take on new meaning when used as part of a regex. + // Also ensure we accept multicharacter separators, in this scenario + createInstance({ + allowNew: true + }) + + paste($('input'), { clipboardData: { + types: ['text/plain', 'Text'], + getData: (type) => 'foo\tbar\r\nbaz\rfam\nbam' + }}) + + sinon.assert.callCount(props.handleAddition, 5) }) - it('can receive tags through paste, respecting delimiters', () => { + it('can receive tags through paste, respecting delimiter chars', () => { // The large range of delimiterChars in the test is to ensure // they don't take on new meaning when used as part of a regex. + // Also ensure we accept multicharacter separators, in this scenario createInstance({ allowNew: true, - delimiterChars: ['^', ',', ';', '.', '\\', '['], + delimiterChars: ['^', ',', ';', '.', '\\', '[', ']X'], suggestions: [{ id: 1, name: 'foo' }] }) paste($('input'), { clipboardData: { types: ['text/plain', 'Text'], - getData: (type) => 'foo,bar;baz^fam\\bam[moo' + getData: (type) => 'foo,bar;baz^fam\\bam[moo]Xark' }}) - sinon.assert.callCount(props.handleAddition, 6) + sinon.assert.callCount(props.handleAddition, 7) }) it('can receive tags through paste, ignoring new tags', () => { From 274fc8137e76ee40791f3df4c7859b33f75a362a Mon Sep 17 00:00:00 2001 From: Andre Mas Date: Thu, 7 Sep 2017 19:37:32 -0400 Subject: [PATCH 08/10] Added artifacts from prepublish --- lib/ReactTags.js | 6 +++++- spec/ReactTags.spec.js | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/ReactTags.js b/lib/ReactTags.js index e0ef103..73396bf 100644 --- a/lib/ReactTags.js +++ b/lib/ReactTags.js @@ -94,6 +94,10 @@ class ReactTags extends React.Component { } } } + } else if (!delimiterChars || delimiterChars.length === 0) { + if (console) { + console.warn('no delimiterChars specified, so ignoring paste operation') + } } } @@ -106,7 +110,7 @@ class ReactTags extends React.Component { if (query || selectedIndex > -1) { e.preventDefault() } - + if (query.length >= this.props.minQueryLength) { // Check if the user typed in an existing suggestion. const match = this.suggestions.state.options.findIndex((suggestion) => { diff --git a/spec/ReactTags.spec.js b/spec/ReactTags.spec.js index d612d44..9491993 100644 --- a/spec/ReactTags.spec.js +++ b/spec/ReactTags.spec.js @@ -410,6 +410,23 @@ describe('React Tags', () => { sinon.assert.callCount(props.handleAddition, 7) }) + it('ignores paste operation, if no delimiterChars are specified', () => { + // The large range of delimiterChars in the test is to ensure + // they don't take on new meaning when used as part of a regex. + // Also ensure we accept multicharacter separators, in this scenario + createInstance({ + allowNew: true, + delimiterChars: [] + }) + + paste($('input'), { clipboardData: { + types: ['text/plain', 'Text'], + getData: (type) => 'foo\tbar\r\nbaz\rfam\nbam' + }}) + + sinon.assert.callCount(props.handleAddition, 0) + }) + it('can receive tags through paste, ignoring new tags', () => { createInstance({ allowNew: false, From 4a6962ceaa27381f305cd25063da273ac7cb7f95 Mon Sep 17 00:00:00 2001 From: Andre Mas Date: Fri, 8 Sep 2017 00:26:54 -0400 Subject: [PATCH 09/10] Allow to access example from second device - Allow example to be access from seconds device, such as allowing to be tested from a mobile - Added 'demiterChars' to example --- example/main.js | 5 ++++- lib/ReactTags.js | 2 +- webpack.config.js | 7 +++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/example/main.js b/example/main.js index 1170719..ffca0c1 100644 --- a/example/main.js +++ b/example/main.js @@ -33,10 +33,13 @@ class App extends React.Component { return (
+ handleAddition={this.handleAddition.bind(this)} + allowNew={true} + /> 
{JSON.stringify(this.state.tags, null, 2)}
diff --git a/lib/ReactTags.js b/lib/ReactTags.js index 73396bf..055c57f 100644 --- a/lib/ReactTags.js +++ b/lib/ReactTags.js @@ -110,7 +110,7 @@ class ReactTags extends React.Component { if (query || selectedIndex > -1) { e.preventDefault() } - + if (query.length >= this.props.minQueryLength) { // Check if the user typed in an existing suggestion. const match = this.suggestions.state.options.findIndex((suggestion) => { diff --git a/webpack.config.js b/webpack.config.js index a7e8bd0..b78e772 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -16,6 +16,13 @@ module.exports = { output: { filename: 'example/bundle.js' }, + devServer: { + disableHostCheck: true, + // listen server to be accessed from another host + // useful for mobile testing + host: "0.0.0.0", + port: process.env.PORT || 8080 + } // resolve: { // alias: { // 'react': 'preact-compat', From 52cad7412d3e1e5270e48b565211b5039fc49020 Mon Sep 17 00:00:00 2001 From: Andre Mas Date: Fri, 8 Sep 2017 00:32:05 -0400 Subject: [PATCH 10/10] Removed 'allowNew' attr from example --- example/main.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/example/main.js b/example/main.js index ffca0c1..3940cb6 100644 --- a/example/main.js +++ b/example/main.js @@ -37,9 +37,8 @@ class App extends React.Component { tags={this.state.tags} suggestions={this.state.suggestions} handleDelete={this.handleDelete.bind(this)} - handleAddition={this.handleAddition.bind(this)} - allowNew={true} - /> + handleAddition={this.handleAddition.bind(this)} + /> 
{JSON.stringify(this.state.tags, null, 2)}