Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding addOnBlur to TagInput #1966

Merged
merged 14 commits into from
Mar 8, 2018
Merged

Adding addOnBlur to TagInput #1966

merged 14 commits into from
Mar 8, 2018

Conversation

kmblake
Copy link
Contributor

@kmblake kmblake commented Dec 22, 2017

Fixes #1412 bullet 5

Checklist

Changes proposed in this pull request:

option to add values on blur (in addition to enter) via addOnBlur prop

Reviewers should focus on:

Screenshot

@kmblake kmblake requested a review from giladgray December 22, 2017 23:46
@blueprint-bot
Copy link

Fix another docs typo

Preview: documentation | landing | table

@kmblake kmblake mentioned this pull request Dec 22, 2017
3 tasks
@blueprint-bot
Copy link

Removing extra dash in doc

Preview: documentation | landing | table

@adidahiya adidahiya changed the base branch from release/1.x to master January 12, 2018 20:38
@adidahiya adidahiya changed the base branch from master to develop January 15, 2018 17:11
@@ -23,6 +23,13 @@ import {
import * as Classes from "../../common/classes";

export interface ITagInputProps extends IProps {
/**
* If true, onAdd will also be invoked when the input loses focus.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think the word also is necessary here, especially since this is the first prop in the list. I think it reads more clearly without it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

// simulate typing input text
wrapper.setProps({ inputProps: { value: NEW_VALUE } });
wrapper.find("input").simulate("change", { currentTarget: { value: NEW_VALUE } });
const fakeEvent = { flag: "yes" };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this flag property for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, yeah I can just call simulate("blur")

wrapper.find("input").simulate("change", { currentTarget: { value: NEW_VALUE } });
const fakeEvent = { flag: "yes" };
wrapper.simulate("blur", fakeEvent);
setTimeout(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Add a comment explaining why setTimeout is necessary. Is this to wait for the focus to change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comment

@blueprint-bot
Copy link

Merge branch 'develop' into kb/tagInput-props

Preview: documentation | landing | table

@blueprint-bot
Copy link

Fixed edge case where inputValue is undefined

Preview: documentation | landing | table

@harrisrobin
Copy link

@kmblake we're super excited to see this land!

Any chance we could get an idea o when this will be merged? :)

Great job 👍

@blueprint-bot
Copy link

Merge branch 'develop' of github.com:palantir/blueprint into kb/tagInput-props

Preview: documentation | landing | table

@blueprint-bot
Copy link

docs tweaks

Preview: documentation | landing | table

@giladgray giladgray merged commit 8ea266a into develop Mar 8, 2018
@giladgray giladgray deleted the kb/tagInput-props branch March 8, 2018 18:41
@giladgray
Copy link
Contributor

@harrisrobin it'll be in the next 2.0 release 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants