From 35c045b77782ad7f15f43925e3c21621003775e6 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Fri, 30 Jun 2017 14:53:51 -0700 Subject: [PATCH 1/6] Change onAdd prop to return a boolean --- packages/labs/src/tag-input/tagInput.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/labs/src/tag-input/tagInput.tsx b/packages/labs/src/tag-input/tagInput.tsx index 6a1f2ebb2c..fd46cccec8 100644 --- a/packages/labs/src/tag-input/tagInput.tsx +++ b/packages/labs/src/tag-input/tagInput.tsx @@ -18,9 +18,9 @@ export interface ITagInputProps extends IProps { /** * Callback invoked when a new tag is added by the user pressing `enter` on the input. * Receives the current value of the input field. New tags are expected to be appended to - * the list. + * the list. If the provided function returns `false`, the new tag will _not_ be added. */ - onAdd?: (value: string) => void; + onAdd?: (value: string) => boolean; /** * Callback invoked when the user clicks the X button on a tag. From 89a5100b225583c2b30c28380197e4e4ae972e84 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Fri, 30 Jun 2017 14:54:56 -0700 Subject: [PATCH 2/6] Clear input only if onAdd returns true --- packages/labs/src/tag-input/tagInput.tsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/labs/src/tag-input/tagInput.tsx b/packages/labs/src/tag-input/tagInput.tsx index fd46cccec8..1abcbb0425 100644 --- a/packages/labs/src/tag-input/tagInput.tsx +++ b/packages/labs/src/tag-input/tagInput.tsx @@ -18,7 +18,10 @@ export interface ITagInputProps extends IProps { /** * Callback invoked when a new tag is added by the user pressing `enter` on the input. * Receives the current value of the input field. New tags are expected to be appended to - * the list. If the provided function returns `false`, the new tag will _not_ be added. + * the list. + * + * If the provided function returns `false`, the input will not be cleared. This is useful, + * for example, if the provided `value` is somehow invalid and should not be added as a tag. */ onAdd?: (value: string) => boolean; @@ -167,8 +170,10 @@ export class TagInput extends AbstractComponent const { selectionEnd, value } = event.currentTarget; if (event.which === Keys.ENTER && value.length > 0) { // enter key on non-empty string invokes onAdd - this.setState({ inputValue: "" }); - Utils.safeInvoke(this.props.onAdd, value); + const shouldClearInput = Utils.safeInvoke(this.props.onAdd, value); + if (shouldClearInput) { + this.setState({ inputValue: "" }); + } } else if (selectionEnd === 0 && this.props.values.length > 0) { // cursor at beginning of input allows interaction with tags. // use selectionEnd to verify cursor position and no text selection. From df0624f048ff13e70729c3b2b0e63ab2c9bc40b6 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Fri, 30 Jun 2017 15:13:54 -0700 Subject: [PATCH 3/6] Add tests --- packages/labs/test/tagInputTests.tsx | 48 ++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/packages/labs/test/tagInputTests.tsx b/packages/labs/test/tagInputTests.tsx index d22cb36680..14d91a41b7 100644 --- a/packages/labs/test/tagInputTests.tsx +++ b/packages/labs/test/tagInputTests.tsx @@ -6,7 +6,7 @@ */ import { assert } from "chai"; -import { mount, shallow } from "enzyme"; +import { mount, shallow, ShallowWrapper } from "enzyme"; import * as React from "react"; import { Intent, Keys, Tag } from "@blueprintjs/core"; @@ -53,27 +53,49 @@ describe("", () => { }); describe("onAdd", () => { + const NEW_VALUE = "new item"; + it("is not invoked on enter when input is empty", () => { - const onAdd = sinon.spy(); - const wrapper = shallow(); - wrapper.find("input").simulate("keydown", { - currentTarget: { value: "" }, - which: Keys.ENTER, - }); + const onAdd = sinon.stub().returns(true); + const wrapper = mountTagInput(onAdd); + pressEnterInInput(wrapper, ""); assert.isTrue(onAdd.notCalled); }); it("is invoked on enter", () => { - const value = "new item"; - const onAdd = sinon.spy(); - const wrapper = shallow(); + const onAdd = sinon.stub().returns(true); + const wrapper = mountTagInput(onAdd); + pressEnterInInput(wrapper, NEW_VALUE); + assert.isTrue(onAdd.calledOnce); + assert.strictEqual(onAdd.args[0][0], NEW_VALUE); + }); + + it("does not clear the input if onAdd returns false", () => { + const onAdd = sinon.stub().returns(false); + const wrapper = mountTagInput(onAdd); + wrapper.setState({ inputValue: NEW_VALUE }); + pressEnterInInput(wrapper, NEW_VALUE); + assert.strictEqual(wrapper.state().inputValue, NEW_VALUE); + }); + + it("clears the input if onAdd returns true", () => { + const onAdd = sinon.stub().returns(true); + const wrapper = mountTagInput(onAdd); + wrapper.setState({ inputValue: NEW_VALUE }); + pressEnterInInput(wrapper, NEW_VALUE); + assert.strictEqual(wrapper.state().inputValue, ""); + }); + + function mountTagInput(onAdd: Sinon.SinonStub) { + return shallow(); + } + + function pressEnterInInput(wrapper: ShallowWrapper, value: string) { wrapper.find("input").simulate("keydown", { currentTarget: { value }, which: Keys.ENTER, }); - assert.isTrue(onAdd.calledOnce); - assert.strictEqual(onAdd.args[0][0], value); - }); + } }); describe("onRemove", () => { From 9c6acf49b309a8ea388791e1bc19e7406139d645 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Fri, 30 Jun 2017 15:43:21 -0700 Subject: [PATCH 4/6] Support boolean OR void for onAdd --- packages/labs/src/tag-input/tagInput.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/labs/src/tag-input/tagInput.tsx b/packages/labs/src/tag-input/tagInput.tsx index 1abcbb0425..20c626ed11 100644 --- a/packages/labs/src/tag-input/tagInput.tsx +++ b/packages/labs/src/tag-input/tagInput.tsx @@ -20,10 +20,11 @@ export interface ITagInputProps extends IProps { * Receives the current value of the input field. New tags are expected to be appended to * the list. * - * If the provided function returns `false`, the input will not be cleared. This is useful, - * for example, if the provided `value` is somehow invalid and should not be added as a tag. + * By default, the input will be cleared after `onAdd` is invoked. However, if the `onAdd` + * function returns `false`, the input will _not_ be cleared. This is useful, for example, + * if the provided `value` is somehow invalid and should not be added as a tag. */ - onAdd?: (value: string) => boolean; + onAdd?: (value: string) => boolean | void; /** * Callback invoked when the user clicks the X button on a tag. @@ -171,7 +172,10 @@ export class TagInput extends AbstractComponent if (event.which === Keys.ENTER && value.length > 0) { // enter key on non-empty string invokes onAdd const shouldClearInput = Utils.safeInvoke(this.props.onAdd, value); - if (shouldClearInput) { + + // strict equal is necessary to ensure the input is cleared whether the return value is + // true or undefined + if (shouldClearInput !== false) { this.setState({ inputValue: "" }); } } else if (selectionEnd === 0 && this.props.values.length > 0) { From 92b17dfa1273a65ec32d53e421577271da5c02f0 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Fri, 30 Jun 2017 15:44:59 -0700 Subject: [PATCH 5/6] Add/update tests --- packages/labs/test/tagInputTests.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/labs/test/tagInputTests.tsx b/packages/labs/test/tagInputTests.tsx index 14d91a41b7..177d2344dd 100644 --- a/packages/labs/test/tagInputTests.tsx +++ b/packages/labs/test/tagInputTests.tsx @@ -56,14 +56,14 @@ describe("", () => { const NEW_VALUE = "new item"; it("is not invoked on enter when input is empty", () => { - const onAdd = sinon.stub().returns(true); + const onAdd = sinon.stub(); const wrapper = mountTagInput(onAdd); pressEnterInInput(wrapper, ""); assert.isTrue(onAdd.notCalled); }); it("is invoked on enter", () => { - const onAdd = sinon.stub().returns(true); + const onAdd = sinon.stub(); const wrapper = mountTagInput(onAdd); pressEnterInInput(wrapper, NEW_VALUE); assert.isTrue(onAdd.calledOnce); @@ -86,6 +86,14 @@ describe("", () => { assert.strictEqual(wrapper.state().inputValue, ""); }); + it("clears the input if onAdd returns nothing", () => { + const onAdd = sinon.stub(); + const wrapper = mountTagInput(onAdd); + wrapper.setState({ inputValue: NEW_VALUE }); + pressEnterInInput(wrapper, NEW_VALUE); + assert.strictEqual(wrapper.state().inputValue, ""); + }); + function mountTagInput(onAdd: Sinon.SinonStub) { return shallow(); } From 77e68be3d7c3fdfe74f73f3f5a2f0bde0b834a4d Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Tue, 4 Jul 2017 13:53:48 -0700 Subject: [PATCH 6/6] Respond to Gilad CR feedback --- packages/labs/src/tag-input/tagInput.tsx | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/labs/src/tag-input/tagInput.tsx b/packages/labs/src/tag-input/tagInput.tsx index 20c626ed11..e54384855f 100644 --- a/packages/labs/src/tag-input/tagInput.tsx +++ b/packages/labs/src/tag-input/tagInput.tsx @@ -20,9 +20,9 @@ export interface ITagInputProps extends IProps { * Receives the current value of the input field. New tags are expected to be appended to * the list. * - * By default, the input will be cleared after `onAdd` is invoked. However, if the `onAdd` - * function returns `false`, the input will _not_ be cleared. This is useful, for example, - * if the provided `value` is somehow invalid and should not be added as a tag. + * The input will be cleared after `onAdd` is invoked _unless_ the callback explicitly + * returns `false`. This is useful if the provided `value` is somehow invalid and should + * not be added as a tag. */ onAdd?: (value: string) => boolean | void; @@ -172,9 +172,7 @@ export class TagInput extends AbstractComponent if (event.which === Keys.ENTER && value.length > 0) { // enter key on non-empty string invokes onAdd const shouldClearInput = Utils.safeInvoke(this.props.onAdd, value); - - // strict equal is necessary to ensure the input is cleared whether the return value is - // true or undefined + // only explicit return false cancels text clearing if (shouldClearInput !== false) { this.setState({ inputValue: "" }); }