Review Request 126597: Notificatioins rework

[David Edmundson]

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



applets/notifications/package/contents/ui/configNotifications.qml (line 63)
<https://git.reviewboard.kde.org/r/126597/#comment61850>

    you don't need this
    
    you always change  screenPositionSelector.selectedPosition
    
    which means we always rebind
            plasmoid.nativeInterface.screenPosition = Qt.binding(function() {configurationChanged(); return screenPositionSelector.selectedPosition; });
            
    calling configurationChanged


- David Edmundson


On Jan. 2, 2016, 6:55 a.m., Anthony Fieroni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126597/
> -----------------------------------------------------------
> 
> (Updated Jan. 2, 2016, 6:55 a.m.)
> 
> 
> Review request for Plasma, David Edmundson and Martin Klapetek.
> 
> 
> Bugs: 355069
>     https://bugs.kde.org/show_bug.cgi?id=355069
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> Complete not flying any more, please compete rewrite or do not touch this code any more !
> 
> 
> Diffs
> -----
> 
>   applets/notifications/lib/notificationsapplet.h 5b262f1 
>   applets/notifications/lib/notificationsapplet.cpp 891cdb0 
>   applets/notifications/package/contents/ui/Notifications.qml f479a65 
>   applets/notifications/package/contents/ui/ScreenPositionSelector.qml efff648 
>   applets/notifications/package/contents/ui/configNotifications.qml 95a8e59 
>   applets/notifications/plugin/notificationshelper.h 860a2da 
>   applets/notifications/plugin/notificationshelper.cpp 15b4479 
> 
> Diff: https://git.reviewboard.kde.org/r/126597/diff/
> 
> 
> Testing
> -------
> 
> First of all, setProperty animate position, but with this kid of logic, empty notification window is trying to be shown more than *4* times, cause to be drawn in differet positions (flying through screen). Now position is set at once *setPosition(x,y)* and looks and feel pretty good. Removed pretty *ugly* checks and unneeded functions.
> Second of all, configuration was *totally* broken, not only flying windows, but in default configuration they (*notifications*) apears in compleately wrong positions, mostly on bottom left mixed with bottom right.
> Third of all, check on "Use custom position for the notification popup" then choose position , hit apply. Then uncheck it, the check it again, cause not trigger signal configuration changed.
> At last, all test works as expected, no regressions even more, all on configurations *works* as expected no matter how many time chech for positions is checked/unchecked, every time configuration changed is trigger and position is applied. No wrong positions at all, no flying at all.
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

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