<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/130180/">https://git.reviewboard.kde.org/r/130180/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On luglio 15th, 2017, 7:31 a.m. UTC, <b>Simone Gaiarin</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;">Inviala!</pre>
</blockquote>
<p>On luglio 15th, 2017, 9:01 a.m. UTC, <b>Luigi Toscano</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;">You can't "ship it" you own review. "Ship it" is marked by other reviewers who thinks that this should go in.
Do you mean to say that you don't have a developer account and someone else should push this?</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'm not very familiar with review board. Indeed I don't have a developer account. I thought that once approved by clicking ship it it would go through, but then I realized what "ship it" is for, so I was just waiting for someone to push it. So yes, I need someone with a developer account to push it please.</p></pre>
<br />
<p>- Simone</p>
<br />
<p>On luglio 12th, 2017, 6:54 p.m. UTC, Simone Gaiarin 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 KDE Frameworks.</div>
<div>By Simone Gaiarin.</div>
<p style="color: grey;"><i>Updated Lug. 12, 2017, 6:54 p.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=359233">359233</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</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 current "open with" dialog implementation does not follow the KDE principle "Simple by default, powerful when needed" for the following reasons:
- The "run in terminal" and "keep terminal open" options are advanced options and should not be exposed by default
- The primary goal of the dialog should be to select an application from the app tree, running command is an advanced feature</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">My patch changes the behavior as follow:</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Put the two options in a KCollapsibleComboBox collapsed by default</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">The user can expand it only if he needs to use a command line command</li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Implementation details:
- When the KCollapsibleComboBox is clicked it is expanded upward keeping the dialog size fixed and compressing the treeview, I'm not sure this is the best approach, but to make it expand downwards we need to fix the size of the dialog with setSizeConstraint(QLayout::SetFixedSize); which may not be desiderable. Maybe there is a way to keep the dialog resizable and expand the combobox downwards, but I couldn't find it.
- I've increased the vertical size of the dialog (which I think it was too small) also to accomodate the upward expansion which otherwise would make the app tree almost disappear</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Relevant discussions:
https://bugs.kde.org/show_bug.cgi?id=359233
https://forum.kde.org/viewtopic.php?f=285&t=131014</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/widgets/kopenwithdialog.cpp <span style="color: grey">(b28c5b05186bc77413524c99ef61b7967b0ec8b9)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/130180/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>