Bug 311413 - The close button on notification popup does not remove the notification from history
Summary: The close button on notification popup does not remove the notification from ...
Status: RESOLVED FIXED
Alias: None
Product: plasma4
Classification: Plasma
Component: notifications (show other bugs)
Version: 4.10.0
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Marco Martin
URL:
Keywords:
: 311436 313085 313922 315252 317684 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-12-09 14:21 UTC by Kai Uwe Broulik
Modified: 2013-08-29 10:19 UTC (History)
15 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.10.3


Attachments
A patch for LastNotificationPopup.qml of /usr/share/kde4/apps/plasma/plasmoid/org.kde.notifications/contents/ui/ (6.35 KB, patch)
2012-12-26 16:49 UTC, Leszek Lesner
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kai Uwe Broulik 2012-12-09 14:21:08 UTC
When I press the X button on the notification popup, the popup just closes, leaving the notification there. Before 4.10 the notification was dismissed (ie. removed) which is imho the better behavior, otherwise my notifications get cluttered quite easily. When I click the notification it closes but when I explicitly press the hard-to-hit X it should know I intend to delete/dismiss it.

Reproducible: Always

Steps to Reproduce:
1. kdialog --passivepopup "test"
2. Click the X on the resulting popup
3.
Actual Results:  
The notification closes but stays in the history

Expected Results:  
The notification is removed
Comment 1 Christoph Feck 2012-12-09 21:46:57 UTC
*** Bug 311436 has been marked as a duplicate of this bug. ***
Comment 2 Leszek Lesner 2012-12-25 10:12:04 UTC
I can confirm this and also add that after a copy process there are 3 popups not telling me that the copy process suceeded. If I click "X" on one popup another one appears. I don't know really this is a seperate bug or related to this one. 
(on KDE 4.10 RC - 4.9.95)

Here a video showing the 3 popups after a copy dialog and the still in place notification in the menu after closing all popups. 

http://youtu.be/mMGx6FuH5IY
Comment 3 Kai Uwe Broulik 2012-12-25 14:21:27 UTC
Yep, confirmed. Actually it's not another dialog, it's just the same dialog moving and resizing a bit.
From what I can tell this happens when the dialog is not attached to the edge of the screen (ie. you have so many tray icons that the notification widget moves too far to the left).

So you click the notification, then it moves to touch the corner, you click again, then it shrinks, and you click again and it collapses, and then you have to open the history again and delete it. C'mon, srlsly?!
Comment 4 Kai Uwe Broulik 2012-12-25 14:28:17 UTC
I suspect that hackery to be able to drag the notifications around to be responsible.
So, you press the close button, then it gets the signal "Hey! User wants to drag me around" and it reorders itself because it is so close to the screen corner (or whatever) and then the mouse moves outside the close area and nothing happens.
Imho moving the notification wouldn't need to be possible from the notifications controls.
Comment 5 Leszek Lesner 2012-12-26 16:46:23 UTC
I had some time to work on that bug today and came up with a nice little fix that fixes the issue that messages won't delete and thanks to your detailed description Kai I also managed to fix the issue that the popup rearranges. 
I changed the behavior of that completely and added a move button on the top left corner which on a press and hold and move than allows to place the popup wherever you want. 
I also fixed the height of the popup, because I noticed that when you have three action buttons in the right column, they overlap the close button. (You can see this on a low harddrive space warning popup)
Here is a little video (without audio and a little bit choppy as I recorded it on my old netbook): http://youtu.be/6fuNsRY5ecI
Comment 6 Leszek Lesner 2012-12-26 16:49:05 UTC
Created attachment 76023 [details]
A patch for LastNotificationPopup.qml of /usr/share/kde4/apps/plasma/plasmoid/org.kde.notifications/contents/ui/

Here is my patch.
Comment 7 Kai Uwe Broulik 2012-12-26 16:57:43 UTC
Go ahead and put it on https://git.reviewboard.kde.org/ otherwise it is likely to get lost.
I put a more simplistic patch there https://git.reviewboard.kde.org/r/107908/ but yours is probably more a fix than a workaround like mine :)
Comment 8 Leszek Lesner 2012-12-26 17:01:30 UTC
Btw. here the correct link to the video: http://www.youtube.com/watch?v=9jKnzFg08NI
Comment 9 Leszek Lesner 2012-12-26 17:18:46 UTC
Sry Kai I don't have an account for the reviewboard and I really don't know how to upload a patch there.
Comment 10 Kai Uwe Broulik 2012-12-26 17:23:55 UTC
I'd like to put it there on behalf of you if you'd allow me to. :-)
Comment 11 Leszek Lesner 2012-12-26 17:26:16 UTC
Yeah that would be good ;) Please feel free to do so.
Comment 12 Christoph Feck 2013-01-17 13:40:12 UTC
*** Bug 313085 has been marked as a duplicate of this bug. ***
Comment 13 Mike Vaughn 2013-01-20 16:19:46 UTC
Just chiming in to say this still occurs in 4.10-RC3 (using packages from ppa:kubuntu-ppa/beta). It'd be great to see this fixed before release, as this is really annoying. :\
Comment 14 Jekyll Wu 2013-02-08 20:05:37 UTC
*** Bug 313922 has been marked as a duplicate of this bug. ***
Comment 15 Bartosz Brachaczek 2013-02-08 20:08:59 UTC
*** This bug has been confirmed by popular vote. ***
Comment 16 Jekyll Wu 2013-02-16 02:36:15 UTC
*** Bug 315252 has been marked as a duplicate of this bug. ***
Comment 17 Eric Donkersloot 2013-03-26 16:01:21 UTC
What's the current state of fixing this bug?
Comment 18 Woodsman 2013-03-26 22:11:15 UTC
Is there any chance this will be fixed in 4.10.2?
Comment 19 Christoph Feck 2013-04-01 14:41:49 UTC
*** Bug 317684 has been marked as a duplicate of this bug. ***
Comment 20 Eric Donkersloot 2013-04-04 14:20:59 UTC
Is anybody actually working on implementing a fix for this bug? Or is it not assigned to the correct person?
Comment 21 Stephen Gallagher 2013-04-24 16:55:23 UTC
Can we get a status update on this? It's a particularly annoying little bug...
Comment 22 Christian (Fuchs) 2013-04-24 17:24:54 UTC
I might be wrong on this, but the patch tackles something completely different than the title of this bug. 

onClicked: {
   lastNotificationPopup.visible = false

This looks like this behaviour is fully on purpose, even though from a usability point of view I can't see why. It should be removed from the list containing the notifications, but then LastNotificationPopup defines and uses its own list with only the currenty showing notifications, not the "history". It has to be removed from that list.
Comment 23 Leszek Lesner 2013-04-24 17:33:33 UTC
See my first comment. The patch also addresses this. 
But the patch itself is outdated currently. 
We ship a new patch in Neptune currently. 
When I have time again I will download the kde svn/git (whatever) and build the patch against this again. So that upstream can accept it. 
If you are eager to test my fix yourself just add this line in the onClicked Handler, so it looks like this:

onClicked: {
   lastNotificationPopup.visible = false
   notificationsModel.remove((notificationsView.count-1)-notificationsView.currentIndex)

...
Comment 24 Christian (Fuchs) 2013-04-24 18:06:37 UTC
Hi Leszek, 

this already looks a lot better, debatable is imo whether  lastNotificationPopup.visible = false  really should be done if there are more than one current notifications. In this case I think it should remove it from both the notificationsModel and the notificationsView  and flick to the next one. 

Maybe there could be a button to dismiss all of them, similar to how it is offered in the history view. 

But this already improves things, thanks.
Comment 25 Thomas Tanghus 2013-04-24 18:56:00 UTC
(In reply to comment #23)
> onClicked: {
>    lastNotificationPopup.visible = false
> notificationsModel.remove((notificationsView.count-1)-notificationsView.
> currentIndex)

Thank you!!! This has been a serious annoyance and your patch seems to fix it :)
Comment 26 Marco Martin 2013-04-24 18:56:46 UTC
Kai: your patch seems fine

Leszek: quickly looking at the patch is probably fine, but i would really rather prefer it goes trough the git.reviewboard.kde.org process, patches posted here pretty much get lost
Comment 27 Marco Martin 2013-04-29 18:30:06 UTC
Git commit e0366c81f64156476afaf0c81eec3a7e5eb6328f by Marco Martin.
Committed on 29/04/2013 at 20:26.
Pushed by mart into branch 'master'.

remove from history when x is pressed
FIXED-IN:4.10.3

M  +1    -0    plasma/generic/applets/notifications/contents/ui/LastNotificationPopup.qml

http://commits.kde.org/kde-workspace/e0366c81f64156476afaf0c81eec3a7e5eb6328f
Comment 28 Marco Martin 2013-04-29 18:30:07 UTC
Git commit c57c4def81f78e9a70f31e5681e81aa741a02ca2 by Marco Martin.
Committed on 29/04/2013 at 20:26.
Pushed by mart into branch 'KDE/4.10'.

remove from history when x is pressed
FIXED-IN:4.10.3

M  +1    -0    plasma/generic/applets/notifications/contents/ui/LastNotificationPopup.qml

http://commits.kde.org/kde-workspace/c57c4def81f78e9a70f31e5681e81aa741a02ca2