[TagInput] onAdd can return false to prevent clearing input #1309
Conversation
cmslewis
changed the title
| this.setState({ inputValue: "" }); | ||
| Utils.safeInvoke(this.props.onAdd, value); | ||
| const shouldClearInput = Utils.safeInvoke(this.props.onAdd, value); | ||
| if (shouldClearInput) { |
There was a problem hiding this comment.
Let's add a switch in the examples for "Unique values" 👍
Can also come as a follow up PR, happy to take care of this
There was a problem hiding this comment.
I'm iffy on adding a "unique values" switch, because it's doesn't correspond to out-of-the-box behavior. Would love to figure out a good way to communicate how to do that in a future PR though. Maybe we could just include a code snippet similar to NumericInputs section on uncontrolled usage:
http://blueprintjs.com/docs/#core/components/forms/numeric-input.uncontrolled-mode
There was a problem hiding this comment.
That's fair, I mean I guess the documentation speaks for itself now, we really were lacking this return false capability
| */ | ||
| onAdd?: (value: string) => void; | ||
| onAdd?: (value: string) => boolean; |
There was a problem hiding this comment.
What about boolean | void, so that you can return both true or nothing if you want the tag to be added?
There was a problem hiding this comment.
Ah yep, missed that. Fixed. 👍
Add testsPreview: documentation |
Add/update testsPreview: documentation |
| * | ||
| * 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. |
There was a problem hiding this comment.
shorter, same idea:
The input will be cleared after
onAddis invoked unless the callback explicitly returnsfalse. This is useful if the providedvalueis somehow invalid and should not be added as a tag.
| this.setState({ inputValue: "" }); | ||
| Utils.safeInvoke(this.props.onAdd, value); | ||
| const shouldClearInput = Utils.safeInvoke(this.props.onAdd, value); | ||
|
|
There was a problem hiding this comment.
unnecessary whitespace as next block is directly related to variable definition: continuing the thought.
| 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 |
There was a problem hiding this comment.
how about pithy "only explicit return false cancels text clearing"
giladgray
changed the title
cmslewis
force-pushed
the
cl/taginput-1277
branch
from
10ae44c to
77e68be
Compare
|
@giladgray ready for re-review |
Respond to Gilad CR feedbackPreview: documentation |
Merge remote-tracking branch 'origin/master' into cl/taginput-1277Preview: documentation |
Fixes #1277
Checklist
Changes proposed in this pull request:
TagInputonAddnow returns aboolean, that, iffalse, will cause the input NOT to be cleared.valueis somehow invalid and should not be added as a tag (though whether or not the tag is added is the consumer's concern, due to the strictly controlled nature ofTagInput).Reviewers should focus on:
Does this behavior seem reasonable?