-
Notifications
You must be signed in to change notification settings - Fork 0
Adding required field designation #12
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
base: paddings_for_formatting_tf
Are you sure you want to change the base?
Adding required field designation #12
Conversation
antonikochet
commented
Mar 7, 2024
- adding required field designation and example
| } | ||
| } | ||
|
|
||
| open var requiredText: String? = " *" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should manage that space internally. Its not really obvious for user that he/she should take care of it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to private property
| } | ||
| } | ||
|
|
||
| open var isRequired: Bool = false { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to somehow mention this functionality in README
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we should also think how to reduce amount of properties that serves this 'is required' feature. It's 3 of them now and it feels like a lot.
Maybe we can hardcode * as 'required field' symbol or use error color for coloring. or just use an attributed string. Let's think what would fit better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added to readme. And removed the color for the required field, now errorTintColor will be used
| guard placeholder != nil else { return } | ||
| placeholderLabel.text = placeholder | ||
| guard let placeholder else { return } | ||
| if isRequired, let requiredText { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps we need another idea here, because for some reason this solution has visual glitches during animation. You can compare it by switching to main branch
…been added to the readme