Support "default actions", as specified in [1].
ClosedPublic

Authored by albertvaka on Jan 15 2017, 3:40 PM.

Details

Summary

Default actions are the actions triggered when the notification is simply
clicked, without adding any "action buttons". Until now, with
KNotifications we could only make notifications that go away when clicked,
which might not be what the user expects (see notifications on smartphones
or other desktop environments).

Since KNotifications uses an int->name mapping (instead of string->name),
we need to manually convert between "default" and "0". Interestingly
enough, there was already code to handle the action "0" and emit
activated(), which is documented as "Emitted only when the default
activation has occurred" but in practice was never actually used.

[1] https://people.gnome.org/~mccann/docs/notification-spec/notification-spec-latest.html

Test Plan

Tests pass.

Diff Detail

Repository
R289 KNotifications
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
albertvaka updated this revision to Diff 10183.Jan 15 2017, 3:40 PM
albertvaka retitled this revision from to Support "default actions", as specified in [1]..
albertvaka updated this object.
albertvaka edited the test plan for this revision. (Show Details)
Restricted Application added a project: Frameworks. · View Herald TranscriptJan 15 2017, 3:40 PM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
albertvaka updated this revision to Diff 10185.Jan 15 2017, 3:46 PM

Commited a file by mistake. Still learning how to use arc :)

albertvaka updated this revision to Diff 10186.Jan 15 2017, 3:47 PM

Missing QStringLiteral

As of now in Plasma, the "default" action will appear as another action button, while in Gnome it will behave as the linked spec says: clicking the notification popup triggers the default action. If this is accepted I will propose a patch for making the notifications plasmoid also trigger the default action just by clicking the notification popup.

mart added a reviewer: VDG.Jan 16 2017, 5:15 PM
mart added a subscriber: mart.Jan 16 2017, 5:21 PM

code wise the change makes sense..
that said, this assumes we do an important behavior change in our ui, which i don't think is a good thing, as is disregards completely how our users are used to, in the name of "other platforms do things differently".
so OK for the change, but only if we have considered it very carefully.
moreover, that spec linked there isn't actually, the spec, it's a fork by gnome, becausethey simply don't care about freedesktop, so is not really a part of the cross desktop spec missing, as the spec linked there wasn't really intended to be cross-desktop at all.
I've added VDG in reviewers, hopefully getting a feedback about this.

Question phrased for VDG: for notifications is better the current behavior of dismissing the notification on a click anywhere or to execute the default action if any? (and do not do anything if no action is available)
most important: what could be the impact on existing users? (personally, I think if i would lose this very fast way to dismiss a notification, i would find them way more intrusive and workflow-interrupting than they are now)

I always hated that clicking the notification popup just closed the popup without doing anything. While our users are probably used to always having a visible "Open" or similar button, I think this makes sense to have.

Care needs to be taken that existing notifications don't suddenly behave differently but that moving from actions() to defaultAction() is a conscious and explicit choice in the application that emits the notification.

+1 from me.

Hm, that's not easy to decide. Whether one likes the "click to make go away" behavior or not is highly subjective. I, for one, find it very annoying on e.g. OS when a notification covers something I want to see on the screen and there is no way to make it go away other than waiting. Others find it annoying to have to explicitly click a button to execute the default action.

I would not take mobile OSes as a reference, because their notifications never cover any part of another application (except when they "peek in", but then they go away again very quickly, and even that can be annoying) and can be dismissed easily with a swipe.

Therefore we should look at other desktop OSes for whether there is a de facto standard. How does Unity do it? Pantheon? Budgie?

and there is no way to make it go away other than waiting.

There's still the (x) button in the corner.

Care needs to be taken that existing notifications don't suddenly behave differently but that moving from actions() to defaultAction() is a conscious and explicit choice in the application that emits the notification.

I've already made sure that existing apps behave as they currently do. Even more: apps that decide to move from setActions to setDefaultAction will still display exactly the same way in Plasma, with an action button (until/unless we change the Plasmoid to honor the default action, which we can discuss when I submit the patch for the Plasmoid).

In D4142#77832, @mart wrote:

code wise the change makes sense..
that said, this assumes we do an important behavior change in our ui, which i don't think is a good thing, as is disregards completely how our users are used to, in the name of "other platforms do things differently".

No, there is no change in our UI at all. In Plasma and KDE apps we can decide to never use this feature if we think that the UX is better that way, but IMO this is a generic (not Plasma-specific) framework and it should provide access to the functionality anyway. This is a really common use case for notifications on most platforms, so if we want that some day this framework can be useful on, for example, Android or Gnome, this is something we have to provide. I think that it would be good to adopt it on Plasma and KDE apps, but this is a completely different discussion that we can have on the mailing list.

And no, I don't think that our users will freak out if we start behaving like every other notification system.

Hm, that's not easy to decide. Whether one likes the "click to make go away" behavior or not is highly subjective. I, for one, find it very annoying on e.g. OS when a notification covers something I want to see on the screen and there is no way to make it go away other than waiting. Others find it annoying to have to explicitly click a button to execute the default action.

Same thing: the framework giving access to this feature doesn't mean your app has to use it. Even more: it doesn't even change the behaviour in Plasma! In Plasma the default action will still appear as a button unless we change the plasmoid. This will, however, make it possible to use this feature on desktops that do support default actions.

>>>! In D4142#77861, @colomar wrote:

Hm, that's not easy to decide. Whether one likes the "click to make go away" behavior or not is highly subjective. I, for one, find it very annoying on e.g. OS when a notification covers something I want to see on the screen and there is no way to make it go away other than waiting. Others find it annoying to have to explicitly click a button to execute the default action.

Same thing: the framework giving access to this feature doesn't mean your app has to use it. Even more: it doesn't even change the behaviour in Plasma! In Plasma the default action will still appear as a button unless we change the plasmoid. This will, however, make it possible to use this feature on desktops that do support default actions.

Okay, in that case VG input is not needed for this patch.
As you said, our frameworks are supposed to be useful outside of Plasma, so they should definitely provide the feature.

mart accepted this revision.Jan 16 2017, 8:19 PM
mart added a reviewer: mart.

No, there is no change in our UI at all. In Plasma and KDE apps we can decide to never use this feature if we think that the UX is better that way, but IMO this is a generic (not Plasma-specific) framework and it should provide access to the functionality anyway. This is a really common use case for notifications on most

for the framework part, as i said, the code is OK, so go for it..
but, to me it really strongly begs the question whether we in the end want this exposed in the UI or not.
having the functionality entering the framework is a good occasion to think about it and take a decision.

This revision is now accepted and ready to land.Jan 16 2017, 8:19 PM
This revision was automatically updated to reflect the committed changes.