Skip to content
This repository has been archived by the owner on Sep 4, 2020. It is now read-only.

Set get badge count android #1644

Merged
merged 7 commits into from
Apr 3, 2017

Conversation

rastafan
Copy link
Contributor

@rastafan rastafan commented Mar 21, 2017

Added getApplicationIconBadgeNumber method for Android

Description

Added getApplicationIconBadgeNumber method for retrieving badge count in Android using SharedPreferences to store it

Related Issue

#1639

Motivation and Context

Made to add support to cordova-plugin-badge, which was unable to retrive badge number set by phonegap-plugin-push

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Member

@macdonst macdonst left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me. I left some comments on the review. Just some minor stuff to reduce the amount of code necessary for this feature. Can you please make those changes then I'll merge them in?

@@ -59,6 +59,7 @@
public static final String CONTENT_AVAILABLE = "content-available";
public static final String TOPICS = "topics";
public static final String SET_APPLICATION_ICON_BADGE_NUMBER = "setApplicationIconBadgeNumber";
public static final String GET_APPLICATION_ICON_BADGE_NUMBER = "getApplicationIconBadgeNumber";
Copy link
Member

Choose a reason for hiding this comment

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

Minor point, fix the indentation here.


int badgeCount = getApplicationIconBadgeNumber(getApplicationContext());
PluginResult pluginResult = new PluginResult(PluginResult.Status.OK, badgeCount);
pluginResult.setKeepCallback(true);
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need to keep the callback around? Once you return the badge count you should be done.

callbackContext.sendPluginResult(pluginResult);

}
});
Copy link
Member

Choose a reason for hiding this comment

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

The above can probably be written as:

} else if (GET_APPLICATION_ICON_BADGE_NUMBER.equals(action)) {
            cordova.getThreadPool().execute(new Runnable() {
                public void run() {
                    Log.v(LOG_TAG, "getApplicationIconBadgeNumber");                    
					callbackContext.success(getApplicationIconBadgeNumber(getApplicationContext()));
                }
            });

public static int getApplicationIconBadgeNumber(Context context){
SharedPreferences settings = context.getSharedPreferences("badge", Context.MODE_PRIVATE);
int badge = settings.getInt("badge", 0);
return badge;
Copy link
Member

Choose a reason for hiding this comment

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

No need to create a temporary value here unless you are going to log the badge number. You can just return the value of settings.getInt().

//in order for plugin cordova-plugin-badge to retrive it correctly
SharedPreferences.Editor editor = context.getSharedPreferences("badge", Context.MODE_PRIVATE).edit();
editor.putInt("badge", 0);
editor.apply();
Copy link
Member

Choose a reason for hiding this comment

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

There is some duplicate code here in the if/else block. Why not move the setting of the badge number to after the if/else and do something like:

SharedPreferences.Editor editor = context.getSharedPreferences("badge", Context.MODE_PRIVATE).edit();
editor.putInt("badge", badge > 0 ? badge : 0);
editor.apply();

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or Math.max(badge, 0), for clarity.

@rastafan
Copy link
Contributor Author

sorry there is still a problem, gonna fix it soon

@rastafan
Copy link
Contributor Author

Alright, sorry for the delay and for the mistakes, we were in a rush so we didn't pay much attention to details.

Copy link
Member

@macdonst macdonst left a comment

Choose a reason for hiding this comment

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

Look good except for one accidental deletion of functionality.

public void run() {
Log.v(LOG_TAG, "getApplicationIconBadgeNumber");

callbackContext.success(getApplicationIconBadgeNumber(getApplicationContext()));
Copy link
Member

Choose a reason for hiding this comment

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

Indent one more space and remove extraneous lines before and after.

* Retrives badge count from SharedPreferences
*/
public static int getApplicationIconBadgeNumber(Context context){
SharedPreferences settings = context.getSharedPreferences("badge", Context.MODE_PRIVATE);
Copy link
Member

Choose a reason for hiding this comment

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

Use BADGE constant instead of "badge" string on this line and next line.

public static void setApplicationIconBadgeNumber(Context context, int badgeCount) {
if (badgeCount > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

This block you deleted should not be removed. Please add it back.

} else {
ShortcutBadger.removeCount(context);
}
SharedPreferences.Editor editor = context.getSharedPreferences("badge", Context.MODE_PRIVATE).edit();
Copy link
Member

Choose a reason for hiding this comment

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

Use BADGE constant instead of "badge" string on this and next line.

@macdonst
Copy link
Member

@rastafan still some issues in the PR.

@rastafan
Copy link
Contributor Author

Sorry for the delay, I hope everything's alright now :)

@macdonst macdonst merged commit 630907c into phonegap:master Apr 3, 2017
@macdonst
Copy link
Member

macdonst commented Apr 3, 2017

@rastafan awesome! I've just merged the changes. Thanks for the PR. Sorry for the delay, I was on vacation last week.

@lock
Copy link

lock bot commented Jun 3, 2018

This thread has been automatically locked.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants