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

[TagInput] add separator, onChange, placeholder props #1418

Merged
merged 5 commits into from
Aug 8, 2017

Conversation

giladgray
Copy link
Contributor

Addresses #1412

Changes proposed in this pull request:

  • separator prop allows adding multiple entries at once
  • onChange prop combines onAdd and onRemove for easy controlling (and implements basic array manipulation logic)
  • placeholder prop appears only when values is empty
    • use inputProps.placeholder if you want it to appear all the time
  • 🔥 API BREAK: onAdd now receives string[] instead of single string
    - private handleAdd = (value: string) => this.setState([...this.state.values, value])
    + private handleAdd = (values: string[]) => this.setState([...this.state.values, ...values])

Reviewers should focus on:

  • definitely review the individual commits here: one per added prop (tests included!)
  • how awesome are these props?

@blueprint-bot
Copy link

TagInput placeholder prop appears only when values is empty

Preview: documentation
Coverage: core | datetime

Copy link
Contributor

@cmslewis cmslewis left a comment

Choose a reason for hiding this comment

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

Approving with two tiny touch-up requests.

}
}

/** Invokes `onAdd` and `onChange` accordingly to remove the item at the given index. */
Copy link
Contributor

Choose a reason for hiding this comment

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

onRemove, not onAdd

const value = "one, two, three";
const onAdd = sinon.stub();
const wrapper = mountTagInput(onAdd, { separator: false });
// extra spaces to exercise `\s*` in regexp
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we delete this comment?

@blueprint-bot
Copy link

better comments + value splitting ⚛️

Preview: documentation
Coverage: core | datetime

@blueprint-bot
Copy link

update docs for new props, default separator just ","

Preview: documentation
Coverage: core | datetime

@giladgray giladgray merged commit 11cbdb7 into master Aug 8, 2017
@giladgray giladgray deleted the gg/taginput-better branch August 8, 2017 01:07
@giladgray giladgray mentioned this pull request Aug 8, 2017
8 tasks
@llorca llorca mentioned this pull request Aug 15, 2017
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.

4 participants