<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/101701/">http://git.reviewboard.kde.org/r/101701/</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 21st, 2011, 1:05 p.m., <b>Christoph Feck</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;">What I dislike is the position of the size combo box. Either it should be below the list, or, when above the list, share the space with the preview.
Regarding the "use DPI depended size", what we could do if we use a slider, is to have a small "Revert/Default" button next to the slider, like what the new Locale KCM does. Then we can have both the slider, and a way to use the automatic size.
I really would like to see this in 4.8, so if this feature isn't on the feature plan yet, please add it, if you need more time for changes.
Fredrik, further comments?</pre>
</blockquote>
<p>On September 23rd, 2011, 9:16 a.m., <b>Lukas Sommer</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;">Now it's on the feature list.
About the position: Is it okay whan I move it to below the list?
About the slider: And how do we distinghish between "resolution dependend size" and "manual size, choosen be the slider" in the UI?
The problem is that we don't know the "resolution dependend size". In xcursors, there is a way to get cursor _pixmaps_ in the default size (that is what we are doing do display the icon for the entry "resolution dependend" in the combobox"), but xcursors doesn't provide a way to ask for the cursor size _value_ directly.
Furthermore, the question is: What happens when the user changes the resolution later? The cursor size should adopt automatically - otherwise, this option would not make sense. But if it adopts automatically, IMHO this should be reflected an an own state in the UI.</pre>
</blockquote>
<p>On October 5th, 2011, 7:06 p.m., <b>Lukas Sommer</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;">About the position: Maybe the screenshot is missleading: Above the size combobox, there are the buttons "Get new theme" (GHNS), "Install new theme" and "Remove theme". This is also the reason why the size combobox has such a big width: It adopts to the width of the buttons above in the grid.</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 was indeed confusing, I had to start the current version to see that the menu hides the three buttons. Considering that, the position as shown is probably fine.
Regarding button next to the slider, I was suggesting a compromise between Fredrik's suggestion, and our intention to offer a default (DPI depended) size. I am fine with the combo box, but Fredrik is the module maintainer :)</pre>
<br />
<p>- Christoph</p>
<br />
<p>On September 2nd, 2011, 4:40 p.m., Lukas Sommer 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 KDE Base Apps, KDE Runtime, kdelibs, and Christoph Feck.</div>
<div>By Lukas Sommer.</div>
<p style="color: grey;"><i>Updated Sept. 2, 2011, 4:40 p.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;">X11 mouse cursor themes can contain cursors in multiple sizes, making them pseudo-scalable.
It is yet possible in KDE to configure manually the mouse cursor size (editing kcminput.rc). However, the GUI of the corresponding KControl module didn't provide support to change this. This patch add support for changing the mouse cursor size to the GUI.
This are mostly GUI related changes. The underlying data structure XCursorTheme did yet provide support for choosing different sizes and only needed some adjustments.</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;">Tested locally. Works fine for me. Also when using non-standard font DPI values.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=90444">90444</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kcontrol/input/xcursor/cursortheme.h <span style="color: grey">(586ccba)</span></li>
<li>kcontrol/input/xcursor/cursortheme.cpp <span style="color: grey">(92abea5)</span></li>
<li>kcontrol/input/xcursor/legacytheme.h <span style="color: grey">(846bf9b)</span></li>
<li>kcontrol/input/xcursor/previewwidget.h <span style="color: grey">(f4d2c4e)</span></li>
<li>kcontrol/input/xcursor/previewwidget.cpp <span style="color: grey">(3c264fc)</span></li>
<li>kcontrol/input/xcursor/themepage.h <span style="color: grey">(38ca893)</span></li>
<li>kcontrol/input/xcursor/themepage.cpp <span style="color: grey">(6c9f29a)</span></li>
<li>kcontrol/input/xcursor/themepage.ui <span style="color: grey">(2e38054)</span></li>
<li>kcontrol/input/xcursor/xcursortheme.h <span style="color: grey">(b474086)</span></li>
<li>kcontrol/input/xcursor/xcursortheme.cpp <span style="color: grey">(2ecb9ba)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101701/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/101701/s/248/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/09/02/snapshot3_400x100.png" style="border: 1px black solid;" alt="" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>