<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="https://git.reviewboard.kde.org/r/115611/">https://git.reviewboard.kde.org/r/115611/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hello all,
It's so late but I just noticed this API could be improved quite a bit and I am offering to make the fix/review on time for 2.9. Perhaps also some implementation too.
Examples:
- use KRecentDirs in KoFileDialog::getUsedDir() to use standard locations as in previous versions of calligra (Recent Dirs gorup) instead of a custom File Dialogs config group
- what about non-local URLs in the API? Don't we want to use KIO on Linux/KDE Plasma? Currently I see a 'You can only select local files' message in Sheets
- filters: why is the translations and extension list not used from the mime database on Linux/KDE Plasma?
- terms could follow better those from Qt/KDElibs, e.g. I had no idea what's a dialogName, it's basically KDE's fileClass I think: http://api.kde.org/4.x-api/kdelibs-apidocs/kio/html/namespaceKRecentDirs.html#a1d58a14171d3269ed2c82288b4ca9661
I already pushed a small obvious fix today to master.</pre>
<br />
<p>- Jarosław Staniek</p>
<br />
<p>On March 23rd, 2014, 12:21 a.m. UTC, Yue Liu wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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;">
<tr>
<td>
<div>Review request for Calligra.</div>
<div>By Yue Liu.</div>
<p style="color: grey;"><i>Updated March 23, 2014, 12:21 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
calligra
</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;">When I'm porting Q/KFileDialog thing to KoFileDialogHelper I found there are some design issues so refactored KoFileDialogHelper.
Now it names as KoFileDialog and moved to kowidgets.
Some use cases of file dialog check return value of dialog.exec(), so static methods are abandoned and a getDialog() method is provided to do whatever you want to the QFileDialog object, using static methods also results in parameter list too long.
Some use cases just use name filters instead of mimetype, so filter setter refactored to 3 methods:
setNameFilter(QString), setNameFilters(QStringList), setMimeTypeFilters(QStringList)
Qt 5.2 introduced QFileDialog::setMimeTypeFilters(QStringList) but its not available through static methods, another excuse to abandon static methods.
Added getXXX() methods as convenience for what static methods usually do.</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;">Works on OSX.
But I remember the reason static methods are used is related to some kfiledialog-related bugs on Windows, please test whether KFileDialog is still used when setting UseNativeDialog=true in kdeglobals on Windows.</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>krita/gemini/desktopviewproxy.cpp <span style="color: grey">(489a440)</span></li>
<li>krita/plugins/assistants/RulerAssistant/kis_ruler_assistant_tool.cc <span style="color: grey">(dc4faa2)</span></li>
<li>krita/plugins/extensions/dockers/flipbook/flipbookdocker_dock.cpp <span style="color: grey">(ad95ef6)</span></li>
<li>krita/plugins/extensions/dockers/lut/lutdocker_dock.cpp <span style="color: grey">(bd5a8e3)</span></li>
<li>krita/plugins/extensions/imagesplit/imagesplit.cpp <span style="color: grey">(1928893)</span></li>
<li>krita/plugins/extensions/separate_channels/kis_channel_separator.cc <span style="color: grey">(d6d1769)</span></li>
<li>krita/ui/dialogs/kis_dlg_file_layer.cpp <span style="color: grey">(ec5aa52)</span></li>
<li>krita/ui/dialogs/kis_dlg_preferences.cc <span style="color: grey">(c02577f)</span></li>
<li>krita/ui/kis_image_manager.cc <span style="color: grey">(8aeddd3)</span></li>
<li>krita/ui/kis_node_manager.cpp <span style="color: grey">(25ca964)</span></li>
<li>krita/ui/widgets/KisFlipbookSelector.cpp <span style="color: grey">(9384790)</span></li>
<li>krita/ui/widgets/kis_color_space_selector.cc <span style="color: grey">(73bde3b)</span></li>
<li>libs/main/CMakeLists.txt <span style="color: grey">(ac708c8)</span></li>
<li>libs/main/KoDocument.cpp <span style="color: grey">(70c838c)</span></li>
<li>libs/main/KoFileDialogHelper.h <span style="color: grey">(4afafd4)</span></li>
<li>libs/main/KoFileDialogHelper.cpp <span style="color: grey">(2483bc4)</span></li>
<li>libs/main/KoMainWindow.cpp <span style="color: grey">(11a7248)</span></li>
<li>libs/main/KoOpenPane.cpp <span style="color: grey">(bf5b219)</span></li>
<li>libs/widgets/CMakeLists.txt <span style="color: grey">(01f015b)</span></li>
<li>libs/widgets/KoEditColorSetDialog.cpp <span style="color: grey">(bf4eabe)</span></li>
<li>libs/widgets/KoFileDialog.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/widgets/KoFileDialog.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>sheets/dialogs/CSVDialog.cpp <span style="color: grey">(4e9d7ed)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115611/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>