-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Entry ClearButtonVisibility Handler #564
Entry ClearButtonVisibility Handler #564
Conversation
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.
Looks really clean and nice. Besides the tests you said you are still working on, there was one question more than a change.
// Returns the default 'X' char drawable in the AppCompatEditText. | ||
protected virtual Drawable GetClearButtonDrawable() | ||
=> ContextCompat.GetDrawable(Context, Resource.Drawable.abc_ic_clear_material); |
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.
@PureWeen Just pointing this out. Not sure what the official way to override things are like this. Should it be some mapper or some other registration that can be injected without requiring an override in a new type? We obviously need to be careful with having a bajillion mapper functions for everything. Maybe we need to clarify where the line of mapper/override/parameter is.
Device tests are implemented for both platforms. |
Description of Change
This PR implements ClearButtonVisibility property for iOS and Android.
Implements #379
Additions made
Notes
I needed to implement touch and focus listener to handle button's visibility in Android handler. This implementation might need to be reworked later on when a views or controls have general touch and focus listening mechanism (if they the team has plans to do so).
Device tests are missing (WIP) but functionality work as expected for both iOS and Android.
I don't know if this overriding approach in the handler for custom clear button drawable is correct. Open for suggestions.
PR Checklist