> On Jan. 2, 2016, 9:27 a.m., David Edmundson wrote:
> > 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?
>
> Anthony Fieroni wrote:
> Qt 5.5.1, libxcb 1.11.1 it's the lastest stable release on Arch. I'm not
> reverting the patch, the patch is pretty *ugly* and don't work at all. The
> bugs are many, flying on notifications on screen, configuration not work when
> "Use custom position for the notification popup" is unchecked. Just throw
> some notifications (4 to 5) by "kdialog --passivepopup test" and you will see
> position changing (bottom left instead of bottom right), flying top to
> bottom. Configuration is compleatly broken, just uncheck more than one "Use
> custom position for the notification popup" and you will see that signal
> configurationChanged is not triggered, add position changed (on monitor pic)
> makes things not working at all.
> And yes, "Behavior on y {" animation is bypassed setPosition, not only
> that configNotifications.qml is changed to work correctly, to trigger signal
> every time *correctly*.
>
> Anthony Fieroni wrote:
> setPosition(x,y) prevent from flyings, other thigs are to make
> configuration to work correctly. Try my patch, if has some thing that not
> work i want to know.
As far as we can see, we have 3 changes:
- moving some stuff (back) from C++ to QML
>From what I can tell, this is exactly the same, and does nothing.
- a config fix
+1 (except for the minor comment below)
- removing the animation
It's not really a fix as much as just avoiding the problem. However, if it
fixes things, I'm not against it.
I'll wait for Martin to comment on that.
If we do merge that, I want the Behavior on y code removed as it'd be just dead
code.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126597/#review90434
-----------------------------------------------------------
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
