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

[Martin Klapetek]

> 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.
> 
> Martin Gräßlin wrote:
>     do you still need testing on it?

Well so far I have received none (except einar's) and I actually
consider this a major breakage, so I was thinking about just shipping
it. That way everybody would test without needing to do anything.

The problem is the next minor stable release which is tomorrow if
I'm not mistaken. So not much testing time.


- 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/780d343e/attachment.html>