[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request 126221: Rework the notifications positioning a bit
From: "Mark Gaiser" <markg85 () gmail ! com>
Date: 2015-12-03 10:17:16
Message-ID: 20151203101716.29696.5026 () mimi ! kde ! org
[Download RAW message or body]
--===============4444817730445862511==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
> On dec 2, 2015, 11:36 p.m., Mark Gaiser wrote:
> > I had this issue for quite a while!
> > It's in bug #349669, i have high hopes that this change fixes my case as well :)
> >
> > Will try this out somewhere this weekend.
>
> Martin Klapetek wrote:
> No, unfortunately this will not fix this issue, but it is related.
> It's the same problem, in fact, but as the notifications use manual
> positioning on the screen, it needs a fix in that code first.
>
> Afaiu the problem happens when something positions a window "out of
> bounds", ie. out of screen and/or in panel struts, then something else
> takes it (either KWin or Qt/X11 or PlasmaQuick::Dialog) and puts it
> back within bounds, but with y=0. Unfortunately I don't understand
> window placement enough to precisely pinpoint where/why it goes to y=0.
>
> Kai Uwe Broulik wrote:
> Bug 349669 should have been fixed by \
> https://quickgit.kde.org/?p=plasma-framework.git&a=commit&h=8ce47c5f1e422f9093a8da0e2a548082792bbd1a
>
Hi Kai, thank you for letting me know. I'm definitely going to test that patch :)
- Mark
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126221/#review89064
-----------------------------------------------------------
On dec 3, 2015, 4:11 a.m., Martin Klapetek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126221/
> -----------------------------------------------------------
>
> (Updated dec 3, 2015, 4:11 a.m.)
>
>
> Review request for Plasma.
>
>
> Bugs: 355069
> https://bugs.kde.org/show_bug.cgi?id=355069
>
>
> Repository: plasma-workspace
>
>
> Description
> -------
>
> The notifications popup positioning recently regressed
> by some other changes (looks like Qt) and the popups
> would fly across the screen (bug 355069).
>
> The proper solution is using KWin effect but given how
> close the release is, this needs to be dealt with in a
> different way.
>
> The main problem is calculating the initial popup size
> because as long as the Dialog is invisible, it has an
> incorrect geometry, so it needs to be positioned right
> after it's being displayed. The Dialog however gets the
> sizes even later, so the code now calls a slot from Dialog
> that ensures the sizes are correct before the initial
> placement on screen. It's not ideal but I'm out of ideas
> otherwise. Plus it should be only temporary until the
> KWin effect will replace it.
>
>
> Diffs
> -----
>
> applets/notifications/lib/notificationsapplet.cpp aa50aef
> applets/notifications/package/contents/ui/Notifications.qml 306169c
> applets/notifications/plugin/notificationshelper.h cb9b335
> applets/notifications/plugin/notificationshelper.cpp 7c0dd99
> applets/notifications/lib/notificationsapplet.h dc31e1d
>
> Diff: https://git.reviewboard.kde.org/r/126221/diff/
>
>
> Testing
> -------
>
> I can no longer reproduce notifications flying across
> the screen.
>
>
> Thanks,
>
> Martin Klapetek
>
>
--===============4444817730445862511==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/126221/">https://git.reviewboard.kde.org/r/126221/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On december 2nd, 2015, 11:36 p.m. UTC, <b>Mark \
Gaiser</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I had this issue for quite a while! It's in bug \
#349669, i have high hopes that this change fixes my case as well :)</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Will try this out somewhere this weekend.</p></pre> </blockquote>
<p>On december 2nd, 2015, 11:46 p.m. UTC, <b>Martin Klapetek</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">No, \
unfortunately this will not fix this issue, but it is related. It's the same problem, \
in fact, but as the notifications use manual positioning on the screen, it needs a \
fix in that code first.</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Afaiu the problem happens when \
something positions a window "out of bounds", ie. out of screen and/or in panel \
struts, then something else takes it (either KWin or Qt/X11 or PlasmaQuick::Dialog) \
and puts it back within bounds, but with y=0. Unfortunately I don't understand
window placement enough to precisely pinpoint where/why it goes to y=0.</p></pre>
</blockquote>
<p>On december 3rd, 2015, 9:35 a.m. UTC, <b>Kai Uwe Broulik</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Bug \
349669 should have been fixed by \
https://quickgit.kde.org/?p=plasma-framework.git&a=commit&h=8ce47c5f1e422f9093a8da0e2a548082792bbd1a</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Hi \
Kai, thank you for letting me know. I'm definitely going to test that patch \
:)</p></pre> <br />
<p>- Mark</p>
<br />
<p>On december 3rd, 2015, 4:11 a.m. UTC, Martin Klapetek wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Plasma.</div>
<div>By Martin Klapetek.</div>
<p style="color: grey;"><i>Updated dec 3, 2015, 4:11 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=355069">355069</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">The notifications popup positioning recently regressed \
by some other changes (looks like Qt) and the popups would fly across the screen (bug \
355069).</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">The proper solution is using KWin effect but given how \
close the release is, this needs to be dealt with in a different way.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">The main problem is calculating the initial popup size \
because as long as the Dialog is invisible, it has an incorrect geometry, so it needs \
to be positioned right after it's being displayed. The Dialog however gets the
sizes even later, so the code now calls a slot from Dialog
that ensures the sizes are correct before the initial
placement on screen. It's not ideal but I'm out of ideas
otherwise. Plus it should be only temporary until the
KWin effect will replace it.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I can no longer reproduce notifications flying across \
the screen.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>applets/notifications/lib/notificationsapplet.cpp <span style="color: \
grey">(aa50aef)</span></li>
<li>applets/notifications/package/contents/ui/Notifications.qml <span style="color: \
grey">(306169c)</span></li>
<li>applets/notifications/plugin/notificationshelper.h <span style="color: \
grey">(cb9b335)</span></li>
<li>applets/notifications/plugin/notificationshelper.cpp <span style="color: \
grey">(7c0dd99)</span></li>
<li>applets/notifications/lib/notificationsapplet.h <span style="color: \
grey">(dc31e1d)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126221/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============4444817730445862511==--
[Attachment #3 (text/plain)]
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic