<head><base href="local:///"></head><body data-blackberry-caret-color="#00a8df" style="background-color: rgb(255, 255, 255);"><div name="BB10" id="BB10_response_div" dir="auto" style="width:100%;background:#ffffff; font-size: initial;font-family:"Calibri","Slate Pro","sans-serif";color:#1f497d"><br></div><p id="_signaturePlaceholder" name="BB10" dir="auto" style="marginTop:2px; font-size: initial;font-family:"Calibri","Slate Pro","sans-serif";color:#1f497d;">Sent from my BlackBerry 10 smartphone.</p><table width="100%" style="border-spacing:0px;"> <tbody><tr><td colspan="2"><div id="_persistentHeader" style="border-style: solid none none; border-top-color: rgb(181, 196, 223); border-top-width: 1pt; padding: 3pt 0in 0in; font-family: Tahoma, 'BB Alpha Sans', 'Slate Pro'; font-size: 10pt;"><div><b>From: </b>Thomas Lübking</div><div><b>Sent: </b>Wednesday, January 16, 2013 8:47 AM</div><div><b>To: </b>Kai Uwe Broulik; Thomas Lübking; kde-workspace</div><div><b>Reply To: </b>kde-core-devel@kde.org</div><div><b>Subject: </b>Re: Review Request 108433: [High-dpi issues] Fix color kcm list when<br><br> using big fonts</div></div></td></tr></tbody></table><div id="_persistentHeaderEnd" style="border:none;border-top:solid #babcd1 1pt;"></div><br><div id="_originalContent" style="">
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tbody><tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/108433/">http://git.reviewboard.kde.org/r/108433/</a>
</td>
</tr>
</tbody></table>
<br>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 16th, 2013, 10:10 a.m. UTC, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/108433/diff/1/?file=107425#file107425line747" style="color: black; font-weight: bold; text-decoration: underline;">kcontrol/colors/colorscm.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void KColorCm::setupColorTable()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">744</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">commonColorTable</span><span class="o">-></span><span class="n">setRowHeight</span><span class="p">(</span><span class="n">i</span><span class="p">,</span> <span class="n"><span class="hl">button</span></span><span class="o"><span class="hl">-></span></span><span class="n"><span class="hl">sizeHint</span></span><span class="p"><span class="hl">().</span></span><span class="n"><span class="hl">h</span>eight</span><span class="p"><span class="hl">()</span>);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">747</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">commonColorTable</span><span class="o">-></span><span class="n">setRowHeight</span><span class="p">(</span><span class="n">i</span><span class="p">,</span> <span class="n"><span class="hl">minH</span>eight</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">w/o having checked code it seems the view has a static row height and the last kid would by this set the game.
i'd say minHeight should be determined as qMax(minHeight, btn->sizeHint().height()) and initialized by the font height (given the font is equal for all elements, otherwise needs to be qMax'd in every row as well)
ultimately in a second pass set all row heights, while probably even setting one would be sufficient</pre>
</blockquote>
<p>On January 16th, 2013, 1:29 p.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;">minHeight is calculated by the Varies button which is the biggest element here (font + margin), the font is equal, yes.
Also, all rows are getting a minHeight (it's a for..next loop)</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The problem is that you estimate on pre-"known" data ("varies" could be very tiny by fontsize or locle - not that the current situation would be any correct), so the minHeight should be set after the button (or stacked widget) has been created (so that the biggest element determines the height) and then set afterwards.
By "view has a static row height" i meant the "uniformItemSize" atribute is likely set but it seems that is actually a QTableWidget (lol ;-) so that the last time one calls "setRowHieght()" would determine the height of all rows. (looks much like this on the second screenshot)</pre>
<br>
<p>- Thomas</p>
<br>
<p>On January 16th, 2013, 1:56 a.m. UTC, Kai Uwe Broulik wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tbody><tr>
<td>
<div>Review request for kde-workspace.</div>
<div>By Kai Uwe Broulik.</div>
<p style="color: grey;"><i>Updated Jan. 16, 2013, 1:56 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">
<tbody><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;">This makes the row height of color list depend on the height of the "Varies" button.
The KColorButtons on the other pages also need fixing but this should be done in KColorButton in kdelibs rather than hacking in the "Varies" thing there as well.
(Not sure if I need to delete that PushButton afterwards)</pre>
</td>
</tr>
</tbody></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">
<tbody><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;">Yup, see screenshots.</pre>
</td>
</tr>
</tbody></table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kcontrol/colors/colorscm.cpp <span style="color: grey">(b9b911f)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/108433/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<ul>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/colornormalsize.png">After with normal fonts</a></li>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/colorbigsizebefore.png">Before with huge fonts</a></li>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/colorbigsizeafter.png">After with huge fonts</a></li>
</ul>
</td>
</tr>
</tbody></table>
</div>
<br></div> <!--end of _originalContent --></body>