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

[Martin Gräßlin]

> On Dec. 18, 2015, 8:04 a.m., Martin Gräßlin wrote:
> > 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 Klapetek wrote:
>     You still could test it and see if it does not cause any regressions (which it shouldn't).
>     
>     In other news, I have acquired a second screen and can test real multi-screen stuff, all works fine here.

do you still need testing on it?


- Martin


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


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/20160104/ff0e938b/attachment.html>