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


Snarky comments don't really help acheive anything.

What version of Qt are you running. This was all added to work round a change 
in a late Qt XCB 5.5?

Also could you expand on the bugs you're seeing. You've linked to 355069 which 
is marked as fixed with no additional comments since the commit, which doesn't 
tell me much.

This looks pretty much like it's just a revert of 
4a6f7db0018a2a7366ac7f2ad61fc33f31566c03 
Plus a change from setX/setY to setPosition to skip the  the     "Behavior on y 
{" animation. Is that right?

- 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
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to