'Re: Review Request: Fix panel controller width when running on'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request: Fix panel controller width when running on
From:       Rafael_Fernández_López <ereslibre () kde ! org>
Date:       2011-04-30 10:35:44
Message-ID: 20110430103544.21325.17610 () vidsolbach ! de
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]

> On April 30, 2011, 7:26 a.m., Aaron J. Seigo wrote:
> > nice fix :) thanks for the patch .. please commit and push at your conv=
enience. (committing to the 4.6 branch and cherry-picking to master would b=
e awesome; otherwise, i'll do so when i see the commit)

It's always a pleasure :)

- Rafael Fern=C3=A1ndez

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101254/#review2983
-----------------------------------------------------------

On April 30, 2011, 12:48 a.m., Rafael Fern=C3=A1ndez L=C3=B3pez wrote:
> =

> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101254/
> -----------------------------------------------------------
> =

> (Updated April 30, 2011, 12:48 a.m.)
> =

> =

> Review request for Plasma.
> =

> =

> Summary
> -------
> =

> I have multiple monitors (xrandr --output LVDS1 --auto --output VGA1 --pr=
imary --auto --right-of LVDS1). Left one: 1440x900, Right one: 1920x1080. T=
he plasma panel when I run such command takes as maximum size only a portio=
n of my big screen. I usually dragged the panel to the top, and to the back=
 again, having it resized to my screen width. This has been a very annoying=
 behavior for me. Even restarting plasma the same behavior was being shown =
again.
> =

> This patch fixes this situation. Basically, on the showEvent method we on=
ly need to adapt the maximum size of the panel controller, for afterwards c=
alling syncToLocation, which will trigger some calculations that will end u=
p (with the maximum size properly set before) with the correct size for thi=
s panel.
> =

> Additionally, it has been needed to adapt the syncToLocation method becau=
se of this change. With the values that were being set before (QWIDGETSIZE_=
MAX) when there was the need to update struts (this is, switching from hori=
zontal to vertical [or reverse] layout), the panel controller got huge (ful=
l screen, except panel itself).
> =

> You can check the screenshots attached for visual information of the prob=
lem.
> =

> =

> Diffs
> -----
> =

>   plasma/desktop/shell/panelcontroller.h 9e8297b =

>   plasma/desktop/shell/panelcontroller.cpp 5e0dde1 =

> =

> Diff: http://git.reviewboard.kde.org/r/101254/diff
> =

> =

> Testing
> -------
> =

> Restart plasma with single screen (OK)
> Restart plasma with multiple screens (OK)
> Run xrandr while on single screen with plasma running (OK)
> Move panel around, from one edge to another, forcing to update struts (OK)
> =

> =

> Screenshots
> -----------
> =

> Plasma panel (before patch)
>   http://git.reviewboard.kde.org/r/101254/s/144/
> Plasma panel (after patch)
>   http://git.reviewboard.kde.org/r/101254/s/145/
> =

> =

> Thanks,
> =

> Rafael Fern=C3=A1ndez
> =

>

[Attachment #5 (text/html)]

<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/101254/">http://git.reviewboard.kde.org/r/101254/</a>
  </td>
    </tr>
   </table>
   <br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On April 30th, 2011, 7:26 a.m., <b>Aaron J. Seigo</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;">nice fix :) thanks for the patch .. please \
commit and push at your convenience. (committing to the 4.6 branch and cherry-picking to master \
would be awesome; otherwise, i&#39;ll do so when i see the commit)</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;">It&#39;s always a pleasure :)</pre> <br />

<p>- Rafael Fernández</p>

<br />
<p>On April 30th, 2011, 12:48 a.m., Rafael Fernández López wrote:</p>

<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: \
url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black solid;">  <tr>
  <td>

<div>Review request for Plasma.</div>
<div>By Rafael Fernández López.</div>

<p style="color: grey;"><i>Updated April 30, 2011, 12:48 a.m.</i></p>

<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;">I have multiple \
monitors (xrandr --output LVDS1 --auto --output VGA1 --primary --auto --right-of LVDS1). Left \
one: 1440x900, Right one: 1920x1080. The plasma panel when I run such command takes as maximum \
size only a portion of my big screen. I usually dragged the panel to the top, and to the back \
again, having it resized to my screen width. This has been a very annoying behavior for me. \
Even restarting plasma the same behavior was being shown again.

This patch fixes this situation. Basically, on the showEvent method we only need to adapt the \
maximum size of the panel controller, for afterwards calling syncToLocation, which will trigger \
some calculations that will end up (with the maximum size properly set before) with the correct \
size for this panel.

Additionally, it has been needed to adapt the syncToLocation method because of this change. \
With the values that were being set before (QWIDGETSIZE_MAX) when there was the need to update \
struts (this is, switching from horizontal to vertical [or reverse] layout), the panel \
controller got huge (full screen, except panel itself).

You can check the screenshots attached for visual information of the problem.</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;">Restart plasma with \
single screen (OK) Restart plasma with multiple screens (OK)
Run xrandr while on single screen with plasma running (OK)
Move panel around, from one edge to another, forcing to update struts (OK)</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>plasma/desktop/shell/panelcontroller.h <span style="color: grey">(9e8297b)</span></li>

 <li>plasma/desktop/shell/panelcontroller.cpp <span style="color: grey">(5e0dde1)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/101254/diff/" style="margin-left: 3em;">View \
Diff</a></p>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>

<div>

 <a href="http://git.reviewboard.kde.org/r/101254/s/144/"><img \
src="http://git.reviewboard.kde.org/media/uploaded/images/2011/04/30/plasma-panel_400x100.png" \
style="border: 1px black solid;" alt="Plasma panel (before patch)" /></a>

 <a href="http://git.reviewboard.kde.org/r/101254/s/145/"><img \
src="http://git.reviewboard.kde.org/media/uploaded/images/2011/04/30/plasma-panel-patch_400x100.png" \
style="border: 1px black solid;" alt="Plasma panel (after patch)" /></a>

</div>

  </td>
 </tr>
</table>

  </div>
 </body>
</html>

_______________________________________________
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] 