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

Expose textField and fieldLabel to be styled #40

Closed
wants to merge 2 commits into from

Conversation

dmorrow
Copy link

@dmorrow dmorrow commented Oct 4, 2016

In order to properly style the textField and fieldLabel, they need to be public properties.

@rsattar
Copy link
Owner

rsattar commented Nov 21, 2016

Thanks for the pull request!

I'm leaning towards exposing the textfield and the fieldLabel, but a couple of changes to your pull request would be good:

  • They shouldn't be readwrite properties in CLTokenInputView.h. The internal properties should exist as readwrite, and the properties in the header should be readonly.
  • There's no benefit to exposing CLBackspaceDetectingTextField in the header; the readonly property can just be UITextField

One of the reasons I've been hesitant to make these properties public is because now the developer can start modifying the textfield and fieldLabel in ways that breaks CLTokenInputView. For example, they might make themselves the delegate of the textField, or start changing the size of the fieldLabel.

Another approach to "exposing" these properties would be make styling methods where CLTokenInputView will call the developer's handler block (with the textField or fieldLabel as the parameter in that block), and the developer can provide styling safely there, without exposing the internals of CLTokenInputView.

If you've already moved on from wanting to work on this pull request, that's totally fine; just let me know, and I'll make the changes myself.

@dmorrow
Copy link
Author

dmorrow commented Nov 23, 2016

@rsattar If you can make the changes in the way that you're comfortable with, that's probably for the best. I think that exposing the textField and fieldLabel as parameters in a styling block would have the same issues as exposing them in the .h file. If someone wants to break it, they will find a way.

@dmorrow
Copy link
Author

dmorrow commented Dec 15, 2016

I've approached this from a different direction. See PR #44

@dmorrow dmorrow closed this Dec 15, 2016
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.

2 participants