Skip to content
This repository has been archived by the owner on Dec 23, 2022. It is now read-only.

Add chip's index in callbacks #66

Merged
merged 3 commits into from
Jan 5, 2017

Conversation

zazapeta
Copy link
Contributor

@zazapeta zazapeta commented Jan 5, 2017

Added chip's index in onRequestAdd and onRequestDelete callback on controlled ChipInput

Added chip's index in onRequestAdd and onRequestDelete callback on controlled ChipInput
Copy link
Member

@leMaik leMaik left a comment

Choose a reason for hiding this comment

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

Thank you very much for the PR! 👍
The chip index doesn't make sense in onRequestAdd because chips are just requested to be added and don't have an index, yet. Whatever controlls this could decide to use any index (e.g. prepending the chip or sorting the chips).
If you remove the index from onRequestAdd, I'll merge this.

@zazapeta
Copy link
Contributor Author

zazapeta commented Jan 5, 2017

@leMaik With pleasure. You did a good job with this component.
And you right.
Remove index onRequestAdd : ✓

Copy link
Member

@leMaik leMaik left a comment

Choose a reason for hiding this comment

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

Haha, thank you. :D
Almost there, looks like you missed a handleDeleteChip call in line 244.

Passing index to a missing handleDeleteChip
@zazapeta
Copy link
Contributor Author

zazapeta commented Jan 5, 2017

I hope this time its fine :) ?

@leMaik
Copy link
Member

leMaik commented Jan 5, 2017

@zazapeta Looks fine to me, thank you! 🎉

@leMaik leMaik merged commit 8a8beae into TeamWertarbyte:master Jan 5, 2017
zazapeta added a commit to zazapeta/material-ui-chip-input that referenced this pull request Jan 5, 2017
As the index is passed to the callback (onRequestDelete) : TeamWertarbyte#66
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants