<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/120235/">https://git.reviewboard.kde.org/r/120235/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On September 16th, 2014, 4:08 p.m. UTC, <b>Vishesh Handa</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;">We currently have the following ways of changing the dialog size</p>
<ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">By changing the mainItem size</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">By actually changing the size of the dialog via QML</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Window Managers</li>
</ol>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">(3) is not something that is exposed to the user since the dialog never actually has the border. I think we can safely ignore this one. Also, maybe can explicitly set some window flags to tell the WM to never allow the client to resize the window.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">(2) kind of makes sense except that then maybe we shouldn't have (1). Example -</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">Dialog {
    <span style="color: #A0A000">width:</span> <span style="color: #666666">500</span>
    <span style="color: #A0A000">height:</span> <span style="color: #666666">500</span>

    mainItem <span style="color: #666666">:</span> Rectangle {
        width <span style="color: #666666">:</span> <span style="color: #666666">200</span>
        <span style="color: #A0A000">height:</span> <span style="color: #666666">200</span>
    }
}
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What do you think should be happening in this case? It's not obvious. I propose we make the mainItem responsible for the dialog size. The 'width' and 'height' properties of the Dialog can be made read only.</p></pre>
 </blockquote>




 <p>On September 16th, 2014, 4:18 p.m. UTC, <b>Marco Martin</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;">"is not something that is exposed to the user since the dialog never actually has the border."<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
That's not relevant, I never make assumptions on what is currently exposed to the user or what is the current use case, that can change at any moment.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
That's my policy for plasma-framework, that admittely may not be always followed in the current codebase, but that can always be improved, instead of going on the other side. "if it can happen, it has to be managed".</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In the example, I think we should make sure in this case is Dialog winning (and document that) even if the dialog size would have read only properties, window managers can always decide to resize it regardless.</p></pre>
 </blockquote>





 <p>On September 16th, 2014, 4:56 p.m. UTC, <b>Vishesh Handa</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;">But we can explicily block the dialog class from allowing user based resizes with window flags. No? This way we are clearly defining what we allow. The moment the use case changes we can adapt our code.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If you're still not keen on disabling external dialog resizing until we have a clear use case. We can also possibly do the following - Add an explicit flag as to what should be happening. QQuickView does something similar - It has an explicit ResizeMode. Anway, I would still really prefer us adapating to our current uses cases and making them work well instead of targetting everything when it complicates our code base.</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;">I really want to allow extrenal resize.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
The fact that QQuickView allows only one way or the other is a limitation i never liked, especially because making it work both ways, is not hard at all (and SizeWindowToRootItem is completely useless on desktop systems).</p></pre>
<br />










<p>- Marco</p>


<br />
<p>On September 16th, 2014, 3:52 p.m. UTC, Marco Martin 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 Marco Martin.</div>


<p style="color: grey;"><i>Updated Sept. 16, 2014, 3:52 p.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-framework
</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;">A dialog can be resize for two reasons: the mainItem size changes, or the dialog size changes.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">the first can happen programmatically, caused by the Layout, or just by assigning the width.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">the second can be caused either programmatically, assigning the size of the dialog or externally by the windowmanager, that is the only one theat in the end has the only final control of the window size</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>src/plasmaquick/dialog.cpp <span style="color: grey">(79a871b)</span></li>

 <li>tests/dialog_minWidthHeightRepositioning.qml <span style="color: grey">(37bd622)</span></li>

</ul>

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






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








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