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

Tokenize text field in textFieldDidEndEditing #141

Merged
merged 2 commits into from
May 29, 2020

Conversation

rizwan95
Copy link
Contributor

@rizwan95 rizwan95 commented May 29, 2020

The purpose of this PR is tokenize the text that is in the text field after the text field resigns its first responder status.

@rizwan95
Copy link
Contributor Author

@ricardopereira Please review this PR

@ricardopereira
Copy link
Member

@rizwan95 Thank you for the PR! Could you had a description please explaining the reason of this change?

So, the purpose of this PR is tokenize the text that is in the text field after the text field resigns its first responder status, right?

@rizwan95
Copy link
Contributor Author

rizwan95 commented May 29, 2020

@rizwan95 Thank you for the PR! Could you had a description please explaining the reason of this change?

So, the purpose of this PR is tokenize the text that is in the text field after the text field resigns its first responder status, right?
@ricardopereira
Yes, you are right. Sometimes the user might just type in a tag and not use the delimiter character and switch to another field. In that case, automatic tokenization in the textFieldDidEndEditing should be helpful.

@ricardopereira
Copy link
Member

ricardopereira commented May 29, 2020

@rizwan95 Ok, to avoid breaking the existing apps with WSTagsField, I would suggest to create a new Boolean property called shouldTokenizeAfterResigningFirstResponder and only do that when it's true. By default, the property is false. Agree?

@rizwan95
Copy link
Contributor Author

@rizwan95 Ok, to avoid breaking the existing apps with WSTagsField, I would suggest to create a new Boolean property called shouldTokenizeAfterResigningFirstResponder and only do that then it's true. By default, the property is false. Agree?

@ricardopereira : Yes, that sounds right. So shall we do that modification?

Copy link
Member

@ricardopereira ricardopereira left a comment

Choose a reason for hiding this comment

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

See my comments. Thank you.

@ricardopereira
Copy link
Member

@ricardopereira : Yes, that sounds right. So shall we do that modification?

@rizwan95 Yes please.

@rizwan95
Copy link
Contributor Author

See my comments. Thank you.

@ricardopereira Your suggestion has been integrated :)

@ricardopereira
Copy link
Member

@rizwan95 I don't see them. Did you pushed your changes?

@rizwan95
Copy link
Contributor Author

@ricardopereira Please check now.

Copy link
Member

@ricardopereira ricardopereira left a comment

Choose a reason for hiding this comment

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

Perfect. Thanks

@ricardopereira ricardopereira merged commit 9cf04be into whitesmith:master May 29, 2020
@rizwan95
Copy link
Contributor Author

@ricardopereira that's awesome!

@leojkwan
Copy link

leojkwan commented Jul 21, 2020

strange, pod install on 5.3.1 does not have this property in the source.

@leojkwan
Copy link

Update: when I specify the specific master branch `, :git => 'https://github.com/whitesmith/WSTagsField.git' in my Podfile, the changes are pulled up, might be an issue with this projects's cocoapods trunk versioning?

@ricardopereira
Copy link
Member

@leojkwan This hasn't been released yet. I'll try to make a new version this week. Sorry for the inconvenience.

@leojkwan
Copy link

Thanks @ricardopereira !

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.

None yet

3 participants