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

Android bugfixes and general improvements, as well as updates for Android 8 (Oreo). #136

Merged
merged 4 commits into from
Nov 25, 2017

Conversation

knxrb
Copy link
Contributor

@knxrb knxrb commented Oct 20, 2017

Description:
Pull request for improvements and bugfixes I've come across while using this plugin in our app. Also adds support for Android 8 (Oreo), with the channels and badges, as well as specifying the colour for the notification in Android.

Fixes:

My Changes (Android & Sample projects):

  • Added support for setting the notification channels in Android Oreo and above, with a default fallback channel, and support for enabling vibration, and showing the badge dot above the app icon.
  • Added support for setting the colour for the Android notifications.
  • Added a new option to the Android local notifications to make it attempt to bring focus back to the app when tapped. (Note: This gives best results when the MainLauncher Activity has a LaunchMode of SingleTop.)
  • Added click Intent to Android scheduled local notifications to allow them to re-focus/open the app on tap as well.
  • Added two helpful debug options for Android; one for showing the internal notification id at the beginning of the notification title, and another to display a toast with the internal notification id and result (when a notification is tapped).
  • Made the Android NotificationBuilder code more stable.
  • Changed the notification result to return the Id as well as the Action.
  • Updated the Target Framework Version to 8.0.
  • Updated the Xamarin Forms version from 2.3.0.46-pre3 to 2.3.4.270.
  • Updated the Sample app so it runs with newer versions of Visual Studio for Mac & VS 2017, and shows an example of setting the Android notification colour, etc.

No changes have been made to iOS or UWP, other than the UWP csproj formats (so it loads in VS 2017).

Hopefully this is useful, and if you have any suggestions, queries, or changes, please let me know.

… and above, with a default fallback channel, and support for enabling vibration, and showing the badge dot above the app icon.

- Added support for setting the colour for the Android notifications.
- Added a new option to the Android local notifications to make it attempt to bring focus back to the app when tapped. (Note: This gives best results when the MainLauncher Activity has a LaunchMode of SingleTop.)
- Added click Intent to Android scheduled local notifications to allow them to re-focus/open the app on tap as well.
- Added two helpful debug options for Android; one for showing the internal notification id at the beginning of the notification title, and another to display a toast with the internal notification id and result (when a notification is tapped).
- Made the Android NotificationBuilder code more stable.
- Changed the notification result to return the Id as well as the Action.
- Updated the Target Framework Version to 8.0.
- Updated the Xamarin Forms version from 2.3.0.46-pre3 to 2.3.4.270.
- Updated the Sample app so it runs with newer versions of Visual Studio for Mac, and shows an example of setting the Android notification colour, etc.
- Added descriptions to the new options, and channel options.
@adamped
Copy link
Contributor

adamped commented Oct 20, 2017

thank you @knxrb, I was waiting for someone to come along and help :)

I have noticed all these bugs but been so busy lately with my job and other OSS/community stuff, it's not got off my todo list.

In the future, if you could break these up, into smaller PR's, one per feature, that would be easier for me to check, but I will go through and check this one as is.

Copy link
Contributor

@adamped adamped left a comment

Choose a reason for hiding this comment

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

Can you please rename Colour to Color.
IAndroidChannelOptions and AndroidChannelOptions arent checked in. Can you please check them in.
Can we please remove the Debug Properties for the moment. I want to think more about debug options in the production API.
Everything else looks good.

/// Example: #FF00CC
/// </summary>
/// <value>The hex colour</value>
string HexColour { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll just be changing this to HexColor. Even though I am Australian, I still default to using American English on all names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do, makes sense.


// Debug Help
public bool DebugShowCallbackToast { get; set; } = false;
public bool DebugShowIdInTitle { get; set; } = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Will look into what these are, but I don't want Debug properties as part of the release API. They may be a better way to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem, I'll delete these for now.

.SetAutoCancel(true)
.SetColor(Color.ParseColor(options.AndroidOptions.HexColour));

if (options.AndroidOptions.ForceOpenAppOnNotificationTap)
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to self to check this. By default it should open the notification. Need to make sure this is default, and this option is here to turn off the feature if they desire. Also I try to keep options the same over every platform. Will need to see if this is possible on iOS and UWP.


_androidOptions = androidOptions;
}

public IList<INotification> GetDeliveredNotifications(Activity activity)
public static string GetOrCreateChannel(IAndroidChannelOptions channelOptions)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why static?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is static as the AlarmHandler uses it as well to create the channel for scheduled notifications.

@adamped adamped merged commit f236e02 into EgorBo:master Nov 25, 2017
@adamped
Copy link
Contributor

adamped commented Nov 25, 2017

@knxrb - was merged in, but I did have to make a number of changes.

  1. I added the AllowTapInNotificationCenter property. This has to be a new property, otherwise it would have broken people's existing implementations.
  2. Added the same functionality to iOS and UWP
  3. Added badge option to iOSOptions

Thanks for the PR.

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.

None yet

2 participants