Review Request 126408: [notifications] Refactor the screen handling code

[Martin Gräßlin]

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


I have multi-screen, but didn't see the issue so far in the first place. So I'm unsure whether it makes sense for me to try, as the result might be incorrect.

What you could try is running a nested kwin_wayland with multiple outputs and start plasmashell there. That should help you simulating multi-screen.

- Martin Gräßlin


On Dec. 17, 2015, 10:40 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126408/
> -----------------------------------------------------------
> 
> (Updated Dec. 17, 2015, 10:40 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> If anybody with multi-screen setup could test this,
> that'd be much appreciated.
> 
> ---
> 
> After the 5.5.0/.1 there were quite a few reports about
> notifications having all sorts of wrong positions and
> appearing on the wrong screens and combination of these.
> 
> So I started looking into the code, added a thing here,
> removed a thing there and after a while it turned into
> a small refactor of the screen and position handling
> code.
> 
> This patch does this:
> * moves the screen handling code from the import into
>   the applet baseclass, which can access the containment
>   available screen rect and watch for screen changes
>   
> * fixes the applet config dialog's custom screen position
>   setting which has a bug of always being enabled
>   
> * consolidates duplicated code in the helper import
>   into functions
>   
> * ensures that popups have correct positions when on
>   screen that does not start with y=0 (and x=0 in some
>   cases too)
> 
> 
> Diffs
> -----
> 
>   applets/notifications/lib/CMakeLists.txt 6a76c3a 
>   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/configNotifications.qml 95a8e59 
>   applets/notifications/plugin/CMakeLists.txt 2f2239f 
>   applets/notifications/plugin/notificationshelper.h 860a2da 
>   applets/notifications/plugin/notificationshelper.cpp 15b4479 
> 
> Diff: https://git.reviewboard.kde.org/r/126408/diff/
> 
> 
> Testing
> -------
> 
> I've been testing all sorts of im/possible combinations
> of multi-screen setups, panel movements on screen, between
> screens, screen movements etc. Worked as expected in all
> situations.
> 
> Einar77 also confirms things are working correctly after
> a quick test.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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