<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/125339/">https://git.reviewboard.kde.org/r/125339/</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, 2015, 8:57 p.m. UTC, <b>Thomas Pfeiffer</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;">No objections from the usability side.
About the quesiton what should be the default: Honestly, do we even need the preview mode at all? Why does Ark have to do things which other applications can already do?
My vote would be for killing the preview function alltogether and making "Open" the default. If the preview mode stays, it should stay as default, because nobody would use it if it wasn't the default action</p></pre>
</blockquote>
<p>On September 22nd, 2015, 6:52 a.m. UTC, <b>Ragnar Thomsen</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;">Thanks for your comment. Regarding the Preview action, it opens a read-only viewer using KParts technology. Personally, I can't see the usefullness of it, but maybe some users like it. Arguments in favor of keeping it would be that it might be faster than opening a full external application and that it uses KDE-unique technology (KParts). Not sure if the latter is even a valid argument. I have taken a look at three other file archivers (Winrar, 7-Zip for Windows and GNOME's File-roller). Of those, only Winrar provides an integrated text viewer, while the other two just open the file using the associated app. The internal previewer in Ark is also broken for some mimetypes such as XML (see bug 201162) and adds dependency on KF5KHtml framework which is in Porting Aids (meaning we have to port it to something else at some point).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Do you have any opinion on what the names should be for "Open archive" and "Open file" actions (both are currently named just "Open")?</p></pre>
</blockquote>
<p>On October 4th, 2015, 7:31 p.m. UTC, <b>andreas kainz</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;">I would love to see the preview in the right panel there you have an preview of an text file or whatever when you select an file that would be nice for dolphin too and we don't have to talk what should be default.</p></pre>
</blockquote>
<p>On October 4th, 2015, 11:07 p.m. UTC, <b>Thomas Pfeiffer</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;">Would it be possible to use a whitelist of filetypes for which the preview should be shown, so that it's only used where it works and makes sense?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As for the naming: I'd actually name them "Open File With..." and "Open Archive". Actually, I think the whole main menu should be restructured in order to avoid confusion between actions regarding an archive and actions regarding the seleted file within an archive (the whole concept of archives as "files within other files" leads to quite some confusions in an archive manager's UI).
GNOME has taken the easy way out: They just didn't put the action to open a file within an archive in the main menu at all. This would be in conflict with our HIG, though (never use a context menu as the only way to execute an action), so we need both in the main menu.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The main menu restructuring should not be part of this patch, though, so you should leave it as "Open File With..." and "Open Archive"</p></pre>
</blockquote>
<p>On October 5th, 2015, 9:23 a.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;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Would it be possible to use a whitelist of filetypes for which the preview should be shown, so that it's only used where it works and makes sense?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Dolphin (KIO) has a setting for which files to preview, I would expect it to follow that setting.</p></pre>
</blockquote>
<p>On October 5th, 2015, 4:59 p.m. UTC, <b>Ragnar Thomsen</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;">Thanks for your comments. It is certainly possible to show a preview in the info panel, but I think it would be too small to enable reading the text in text files and scrolling etc. will not be possible.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I have agreed with the other developer that we will keep "preview" as default action for 15.12 release, but provide an option in the settings to allow disabling the previewer and using "open" by default. The configuration setting will be implemented in another RR.</p></pre>
</blockquote>
<p>On October 5th, 2015, 5:29 p.m. UTC, <b>Ragnar Thomsen</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;">Thomas, I added a separator in the context and action menus, so "Preview", "Open File" and "Open File With" are separated from the other actions. Previously, there was no separator in the menus. Is this new separation ok? (Don't know if there are any guidelines for these sort of things)</p></pre>
</blockquote>
<p>On October 5th, 2015, 5:53 p.m. UTC, <b>Elvis Angelaccio</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;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thomas, I added a separator in the context and action menus, so "Preview", "Open File" and "Open File With" are separated from the other actions. Previously, there was no separator in the menus. Is this new separation ok? (Don't know if there are any guidelines for these sort of things)</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><a href="https://techbase.kde.org/Projects/Usability/HIG/Patterns/CommandPatterns#Patterns_for_a_complex_command_structure" style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Here</a> you should find something interesting about 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;">All that the relevant HIG ( https://techbase.kde.org/Projects/Usability/HIG/Menu_Bar ) says about separators is "Add separators between logical groups within a menu.". These are logical groups, so yes, it does make sense.
To further avoid confusion, I'd rename the "File" menu to "Archive".</p></pre>
<br />
<p>- Thomas</p>
<br />
<p>On October 5th, 2015, 5:25 p.m. UTC, Ragnar Thomsen 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 Utils, KDE Usability, Elvis Angelaccio, and Raphael Kubo da Costa.</div>
<div>By Ragnar Thomsen.</div>
<p style="color: grey;"><i>Updated Oct. 5, 2015, 5:25 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="http://bugs.kde.org/show_bug.cgi?id=201162">201162</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=208330">208330</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
ark
</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;">Add an Open action, which opens an archive entry with the associated application. This was previously only possible through the "Open With" action, but this required several clicks. Currently, the Preview action is still the default, i.e. when the user clicks an archive entry the Preview action is called.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">QSignalMapper is used to connect the signals from the Open, Open With and Preview actions to the same slot.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As for Preview, the file is extracted to a QTemporaryDir and then opened. The temporarily extracted file is monitored for changes using QFileSystemWatcher and on change a KMessageBox is opened to query the user if the archive should be updated with the modified file. If the user accepts slotAddFiles() is called.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The previously unimplemented path argument of slotAddFiles() is used to allow for updating a file in a subdirectory within the archive.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Things to consider:
1. Should we set some default keyboard shortcuts for "Open File" and "Open File With"? If yes, which?</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Tested with tar-based, zip, rar and 7z archives.</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>part/ark_part.rc <span style="color: grey">(da04d47)</span></li>
<li>part/part.h <span style="color: grey">(1d733c0)</span></li>
<li>part/part.cpp <span style="color: grey">(8854fdd)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/125339/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/10/05/b0b7c44d-2b40-4772-b8d0-a81f6895518a__open-action-1.png">Context menu</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/10/05/deab8bb1-722c-48b8-bce3-64190afc6a53__open-action-2.png">Action menu</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>