D12293: Add actions to notifications
Kai Uwe Broulik
noreply at phabricator.kde.org
Wed Apr 18 20:09:23 UTC 2018
broulik added inline comments.
INLINE COMMENTS
> notification.cpp:114
> + connect(m_notification, QOverload<unsigned int>::of(&KNotification::activated), this, [this] (unsigned int actionIndex) {
> + Q_EMIT actionTriggered(m_internalId, m_actions[actionIndex]);
> + });
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.
> notification.cpp:128
> if (!m_requestReplyId.isEmpty()) {
> - m_notification->setActions(QStringList(i18n("Reply")));
> + m_actions.prepend(i18n("Reply"));
> connect(m_notification, &KNotification::action1Activated, this, &Notification::reply);
You change `m_actions` after having called `setActions()` above
> notification.cpp:132
>
> +
> +
Whitespace
REPOSITORY
R224 KDE Connect
REVISION DETAIL
https://phabricator.kde.org/D12293
To: nicolasfella, #kde_connect
Cc: broulik, 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/20180418/6a3fc386/attachment.html>
More information about the KDEConnect
mailing list