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

🚸 at least on android, clicking on the notification does not do anything #910

Closed
shankari opened this issue May 21, 2023 · 7 comments · Fixed by e-mission/e-mission-phone#989

Comments

@shankari
Copy link
Contributor

I am 99% sure this used to work on the old trip-end notifications, but it is not currently working with the current cordova plugin.
We get the notification asking us to click to label with an action to "change time"
Clicking on "change time" opens the app to the profile screen.
Clicking on the notification directly does not launch the app; the notification just stays there doing nothing

This not such a huge issue, but it just feels very wrong visecerally. Asking people to click should do something when they do click.

In Spanish In English
Screenshot_1684699115 Screenshot_20230521-125745
@shankari
Copy link
Contributor Author

I can confirm that removing the action does launch the app when the user clicks on the notification.

@JGreenlee, for the workaround, I think we should just remove the action. Allowing people to choose the time is important, but not as important as launching the app when they click on the notification. Forcing them to click another button just to label is bad IMHO since we don't even see the buttons by default (see Spanish screenshot above).

I am tempted to make that one line change (remove the action item) and reintroduce it in the point release.
Thoughts?

@shankari
Copy link
Contributor Author

shankari commented May 21, 2023

Here's the one line hack for this.

diff --git a/www/js/splash/notifScheduler.js b/www/js/splash/notifScheduler.js
index a450a2f3..44e89513 100644
--- a/www/js/splash/notifScheduler.js
+++ b/www/js/splash/notifScheduler.js
@@ -80,7 +80,6 @@ angular.module('emission.splash.notifscheduler',
                     title: scheme.title[localeCode],
                     text: scheme.text[localeCode],
                     trigger: {at: nDate},
-                    actions: 'reminder-actions',
                     data: {
                         action: {
                             redirectTo: 'root.main.control',

@shankari
Copy link
Contributor Author

I also confirmed that, on iOS, clicking on the notification does open the app, even if there are actions.

@shankari
Copy link
Contributor Author

shankari commented May 22, 2023

I will investigate this later today since it may be a native code issue. At worst, we fork and fix.

@JGreenlee
Copy link

I came at it from a few angles, and didn't find anything in the JS that would do the trick. So I do suspect it's something with the plugin

@shankari
Copy link
Contributor Author

Found it! It is indeed in the native code.

Concretely:

https://github.com/bhandaribhumin/cordova-plugin-local-notification-12/blob/17826eae5e0a857a2b4051d30032b4e5c767d2be/src/android/notification/Builder.java#L407

        Action[] actions = options.getActions();
        if (actions != null && actions.length > 0 ) {
          // if actions are defined, the user must click on button actions to launch the app.
          // Don't make the notification clickable in this case
          return;
        }

Added in
bhandaribhumin/cordova-plugin-local-notification-12@0b6aefe

There doesn't seem to be any rationale recorded for the change.

@shankari
Copy link
Contributor Author

I can confirm that if I remove those lines from the native code, clicking on the notification (even with actions) opens the app.
I am going to create a fork with those lines removed and switch to that.

In parallel, I have submitted an issue to the main repo to see if it can be resolved at the root.
bhandaribhumin/cordova-plugin-local-notification-12#9

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 a pull request may close this issue.

2 participants