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