Differential D12293

Add actions to notifications
ClosedPublic
Actions

Authored by nicolasfella on Apr 17 2018, 6:36 PM.

Details

Reviewers

apol

Group Reviewers

KDE Connect

Commits

R224:957975898bfc: Add actions to notifications
R224:e65743c0c163: Add actions to notifications

Summary

When Android sends a list of possible actions add them to the notification. When the action is triggered send a package containing the id and the action back
CCBUG: 366475

Test Plan

Send dummy notification, see the actions, trigger it, look for received package on Android

Diff Detail

Repository

R224 KDE Connect

Branch

notificationactions

Lint

No Linters Available

Unit

No Unit Test Coverage

nicolasfella created this revision.Apr 17 2018, 6:36 PM

Restricted Application added a project: KDE Connect. · View Herald TranscriptApr 17 2018, 6:36 PM

nicolasfella requested review of this revision.Apr 17 2018, 6:36 PM

mtijink added a subscriber: mtijink.Apr 17 2018, 6:50 PM

mtijink added inline comments.

plugins/notifications/notification.cpp
114	Do we need a check/index change here when we add "Reply" here?
204	Leftover debug?
plugins/notifications/notificationsdbusinterface.h
55	I think `key` should be `replyId` or something like that.

broulik added a subscriber: broulik.Apr 18 2018, 8:09 PM

broulik added inline comments.

plugins/notifications/notification.cpp
114	Not sure if that can happen but it looks like you might clear `m_actions` at a point where a notifications still shown and then `m_actions[actionIndex]` would be out of bounds And also, like the previous comment, when "Reply" is added, the index shifts.
128	You change `m_actions` after having called `setActions()` above
132	Whitespace

MatMaul added a subscriber: MatMaul.Jun 3 2018, 5:15 PM

Restricted Application added a subscriber: kdeconnect. · View Herald TranscriptJun 3 2018, 5:15 PM

MatMaul removed a subscriber: MatMaul.Jun 3 2018, 5:16 PM

Can you please look into Kai's comments?

This revision now requires changes to proceed.Jul 26 2018, 5:47 PM

nicolasfella edited the summary of this revision. (Show Details)Sep 26 2018, 11:38 PM

Fix stuff

Harbormaster completed remote builds in B7032: Diff 49364.Jan 12 2019, 11:00 PM

apol accepted this revision.Feb 8 2019, 10:17 PM

This revision is now accepted and ready to land.Feb 8 2019, 10:17 PM

Closed by commit R224:957975898bfc: Add actions to notifications (authored by nicolasfella). · Explain WhyFeb 8 2019, 11:01 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			plugins/notifications/kdeconnect_notifications.json (3 lines)
M			plugins/notifications/notification.h (2 lines)
M			plugins/notifications/notification.cpp (21 lines)
M			plugins/notifications/notificationsdbusinterface.h (1 line)
M			plugins/notifications/notificationsdbusinterface.cpp (18 lines)
M			plugins/notifications/notificationsplugin.h (2 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	32416	4a4e821		Apr 17 2018, 6:36 PM	★	★
Diff 2	49364	0b1fbde	- Fix stuff	Jan 12 2019, 11:00 PM	★	★
Diff 3	51225	e14b3dd	R224:957975898bfc22b17ebfff1321f9c89f753a34c2	Feb 8 2019, 10:54 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
eba8ec53f1d5	52530af21bf3	2ead8d1233e8	Nicolas Fella	Add actions to notifications	Apr 17 2018, 6:33 PM
2ead8d1233e8	143f1e355f21	efbb7c4dc176 4a4e8216aa68	Nicolas Fella	Merge branch 'master' into notificationactions	Apr 17 2018, 6:04 PM
efbb7c4dc176	26a2168fc7f3	bf327e5b8c6c	Nicolas Fella	Work	Apr 1 2018, 12:29 PM