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

[flutter_local_notifications] Add "selectable" property for AndroidNotificationDetails to disable click actions on the notification #2331

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

Conversation

tranleo148
Copy link

This PR introduces a new option to disable click actions on notifications from Flutter. In Android, disabling click actions on a notification is achieved by setting the contentIntent to null when building the notification. Currently, there is no option in Flutter to disable the click action.

To address this, I have added a new selectable property that allows developers to control whether the notification is selectable or not. When selectable is set to false, the contentIntent will be set to null, disabling click actions on the notification.

  • Default Behavior:
    The default value of selectable is true, ensuring that existing behavior is not affected unless explicitly changed.
  • Impact:
    This change is backward-compatible and will not affect notifications unless the new property is utilized.

Copy link
Owner

@MaikuB MaikuB left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Left a couple of comments on there. Could you take a look?

@@ -274,7 +274,10 @@ protected static Notification createNotification(
: notificationDetails.body)
.setTicker(notificationDetails.ticker)
.setAutoCancel(BooleanUtils.getValue(notificationDetails.autoCancel))
.setContentIntent(pendingIntent)
.setContentIntent(
BooleanUtils.getValue(notificationDetails.selectable)
Copy link
Owner

Choose a reason for hiding this comment

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

Compared to other parts of the code, I don't believe you should be using BooleanUtils.getValue(). That is for coalescing null values to false. The null values would happen when introducing new fields/properties and there are scheduled notifications kept in shared preferences. With the changes introduced in the PR, my understanding is that null selectable value be coalesced to true instead of false

/// disabling click actions on the notification.
///
/// The default value is `true`.
final bool selectable;
Copy link
Owner

Choose a reason for hiding this comment

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

  • Can you update the example app to include a scenario that makes use of this?
  • I assume the onSelectNotification callback won't fire. Can you confirm? If so, can you update the API docs to explicitly add mention of this. Some developers may not realise the implication based on reading what's there

@MaikuB
Copy link
Owner

MaikuB commented Oct 13, 2024

@tranleo148 I saw a notification that updated your branch. In case you missed it, I requested changes on this PR a while back. Are you going to be able to take a look?

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