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

Make sure isBadgeCounterSupported returns false on Samsung on Android 8+ #268

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

eirikwah
Copy link

The fix is in SamsungHomeBadger, but: Note that the DefaultBadger also must check for Samsung on Android 8, or it will be used on those devices.

I have tried to do this fix without touching DefaultBadger, but it seems impossible with the current design of the API.

PS: The fix would still be technically valid only with the changes in DefaultBadger and not changing SamsungHomeBadger at all. However, I think the code changes in SamsungHomeBadger should be there anyway to better show that ShortcutBadger is not supported on Samsung devices running Android 8+.

This fixes #266

Also included:

  • Update test app to be able to test isBadgeCounterSupported

Note that the DefaultBadger also must check for Samsung on Android 8, or
it will be used on those devices.

This fixes leolin310148#266
@aMarCruz
Copy link

aMarCruz commented Jul 5, 2018

Please do not merge this without more testing. This PR is blocking ShortcutBadger in all Samsungs 8 and above right?
I think this behavior is the responsibility of the launcher. Evie for example has already added support for O (I will check later).

@eirikwah
Copy link
Author

eirikwah commented Jul 6, 2018

This PR is blocking ShortcutBadger in all Samsungs 8 and above right?

This PR does not block ShortcutBadger on Samsung 8, there is nothing to block (it does not work, or at least did not work at the time this PR was written). SO this is a fix for the consitency of the API: Do not state that Samsung Galaxy 8 on Oreo is supported, when API calls have no effect on any badges.

@leolin310148
Copy link
Owner

@eirikwah please fix the conflict, thank you!

# Conflicts:
#	ShortcutBadger/src/main/java/me/leolin/shortcutbadger/impl/DefaultBadger.java
@eirikwah
Copy link
Author

eirikwah commented Oct 3, 2018

@eirikwah please fix the conflict, thank you!

@leolin310148 Done now. PS: I don't have the development environment set up correctly on my PC anymore, so I did not have the chance to easily test this again after the conflict resolution. However, it was fairly straight-forward.

@geraintwhite
Copy link

Is this going to be merged anytime soon?

@grebulon
Copy link

This caused problems when tested on S8 with Android 9. The badge stopped updating.

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.

Samsung running Android 8 is reported as supported, but is not
5 participants