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

Added keylistner to KProgresshud #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

upendrashah47
Copy link

Added keylistner for doing custom on back press key and other keys too

@@ -37,6 +37,7 @@ android {
dependencies {
compile fileTree(dir: 'libs', include: ['*.jar'])
testCompile 'junit:junit:4.12'
compile 'org.jetbrains:annotations-java5:15.0'
Copy link
Contributor

@tuanna-hsp tuanna-hsp Apr 20, 2018

Choose a reason for hiding this comment

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

Using this type of annotation is generally good for the IDE to spot bugs and output some warnings.
But in this case I think it not so important to add a lib just for a single @nullable. Which IMHO, can be omitted without any problems.

public void setOnKeyListener(OnKeyListener onKeyListener) {
super.setOnKeyListener(onKeyListener);
mOnKeyListener = onKeyListener;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it not necessary to override this method, then save the reference which I don't see being call anywhere.

* @return Current HUD
*/
public KProgressHUD setOnKeyListener(@Nullable DialogInterface.OnKeyListener onKeyListener) {
this.mProgressDialog.setOnKeyListener(onKeyListener);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just this is nice. You don't need to make any changes inside dialog class (I commented below).

@tuanna-hsp
Copy link
Contributor

tuanna-hsp commented Apr 20, 2018

Sorry for late reply. Your PR and the docs-reformatting is welcome.
I have left some comments, take a look when you have time and update the PR if neccesary.

@upendrashah47
Copy link
Author

upendrashah47 commented May 8, 2018 via email

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