Review Request 121360: Rework Plasma's notification positioning code

[Martin Klapetek]

> On Jan. 3, 2015, 9:08 p.m., Kai Uwe Broulik wrote:
> > Overall the notification appearance is much cleaner, they don't overlap anymore and when they're updated they don't just flicker re-appear but fade out and fade in again.
> > However, the vertical positioning is completely off with the notifications dancing around the entire height of the screen from which it never recovers when you had multiple notifications appearing in quick succession.

For the record, this got fixed by Kai. And I'll include it in the patch.


> On Jan. 3, 2015, 9:08 p.m., Kai Uwe Broulik wrote:
> > applets/notifications/plugin/notificationshelper.cpp, line 51
> > <https://git.reviewboard.kde.org/r/121360/diff/2/?file=337844#file337844line51>
> >
> >     Use QScopedPointer

What's wrong with properly used delete?


> On Jan. 3, 2015, 9:08 p.m., Kai Uwe Broulik wrote:
> > applets/notifications/plugin/notificationshelper.cpp, line 85
> > <https://git.reviewboard.kde.org/r/121360/diff/2/?file=337844#file337844line85>
> >
> >     New style connect?

This is a QML signal, I'm not sure that's possible...?


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121360/#review73046
-----------------------------------------------------------


On Jan. 3, 2015, 1:26 a.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121360/
> -----------------------------------------------------------
> 
> (Updated Jan. 3, 2015, 1:26 a.m.)
> 
> 
> Review request for Plasma and Kai Uwe Broulik.
> 
> 
> Bugs: 339732
>     https://bugs.kde.org/show_bug.cgi?id=339732
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> There can easily be situations where the popups could overlap one another or result in strange animations. This patch rewrites the notifications so that all actions such as show/reposition/hide are handled from a one single place and every action is properly queued and protected around, which makes it more robust, more predictive and less chaotic. There's also a slight delay between every action so it's also visually much more cleaner and easier to see what's going on. 
> 
> 
> Diffs
> -----
> 
>   applets/notifications/package/contents/ui/NotificationPopup.qml 4491230 
>   applets/notifications/plugin/notificationshelper.h af8f6fa 
>   applets/notifications/plugin/notificationshelper.cpp 425f0d6 
> 
> Diff: https://git.reviewboard.kde.org/r/121360/diff/
> 
> 
> Testing
> -------
> 
> Tested whole day plus stress-tested with something like for i in {1..200}; do notify-send "$i - $RANDOM" "$RANDOM sdf sdf sdfwefhsdjfnskdfbkwefnos igodsfgn sodifgj asodfgnsdlfgdf g"; done executed from 4 terminals at once, all works fine and as expected.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150105/cabffbbb/attachment.html>