D12294: Send notification actions

[Matthijs Tijink]

mtijink added a comment.


  I think we should also send the action icon. That might be hard at the moment though, because we can only send one payload.

INLINE COMMENTS

> NotificationsPlugin.java:77
>      private Map<String, RepliableNotification> pendingIntents;
> +    private Map<String, List<Notification.Action>> actions;
>      private boolean serviceReady;

Maybe it's good to refactor this into `Map<String, RemotelyAccessedNotification>` or similar.

`RemotelyAccessedNotification` can then contain the id, the available actions and reply intent.

> NotificationsPlugin.java:264
> +                actions.put(key, new LinkedList<Notification.Action>());
> +                JSONArray jsonArray = new JSONArray();
> +                for (Notification.Action action : notification.actions) {

I think renaming to `actionsJson` is clearer.

> NotificationsPlugin.java:267
> +                    if (null == action.title)
> +                        break;
> +                    Log.d(TAG, action.title.toString());

Maybe if a single title is missing, don't send any actions? Otherwise, this can be confusing.

> NotificationsPlugin.java:273
> +                np.set("actions", jsonArray);
> +                Log.d(TAG, np.getString("actions"));
> +            }

Double `Log` (with line 268), I'd remove at least one of them.

> NotificationsPlugin.java:508
> +                } catch (PendingIntent.CanceledException e) {
> +                    e.printStackTrace();
> +                }

Something like `Log.e(TAG, "Firing action for notification failed", e)` is better in general (colors the stacktrace etc., actually labels it as an error).

REPOSITORY
  R225 KDE Connect - Android application

REVISION DETAIL
  https://phabricator.kde.org/D12294

To: nicolasfella, #kde_connect
Cc: mtijink, #kde_connect, yannux, Danial0_0, johnq, Pitel, adeen-s, SemperPeritus, ahmedbesbes, ndavis, daniel.z.tg, jeanv, seebauer, bugzy, MayeulC, menasshock, ach, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20180417/f209a1a8/attachment-0001.html>