<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/105972/">http://git.reviewboard.kde.org/r/105972/</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;">Thanks for the patch and for your interest in improving Dolphin!
Some questions:
1. I don't know much about QAccesible. Could you describe briefly what kind of functionality your patch adds to Dolphin?
2. Please follow the coding style. In particular, always use {...} after 'if' and similar statements. Look at existing Dolphin code for reference or look at
http://techbase.kde.org/Policies/Kdelibs_Coding_Style
3. KItemListSelectionManager does intentionally not depend on KItemListView and KItemListController, and it should stay this way. Please remove the corresponding includes from kitemlistselectionmanager.cpp. The accessibiliy update that you do in KItemListSelectionManager::setCurrentItem() now could also be done in the view's slot which is invoked by the selection manager's currentChanged() signal.
4. The new files you are proposing to add contain lots of commented-out code and are therefore not easy to review. Please fix that before uploading a new version of your patch.
</pre>
<br />
<p>- Frank</p>
<br />
<p>On August 11th, 2012, 9:34 a.m., Amandeep Singh 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 Dolphin, KDE Base Apps and KDE Accessibility.</div>
<div>By Amandeep Singh.</div>
<p style="color: grey;"><i>Updated Aug. 11, 2012, 9:34 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">
<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;">Added Accessibility Interfaces for Dolphin Views & Widgets, to make it accessible.
2 New files added in dolphin/ src/ kitemviews/ kitemlistviewaccessible.* that contain the three new classes.</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;">Focus-tracking tested with KMag / KWin. </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>dolphin/src/CMakeLists.txt <span style="color: grey">(5c1a6dad58cb579ee85731d4bfa0ebe9a6a1bea4)</span></li>
<li>dolphin/src/kitemviews/kitemlistcontainer.cpp <span style="color: grey">(5500851c8c92c564bf3130c66198cea9b61eb8c7)</span></li>
<li>dolphin/src/kitemviews/kitemlistcontroller.cpp <span style="color: grey">(88f5d9f7cff1614fb8595b470d46916584710a27)</span></li>
<li>dolphin/src/kitemviews/kitemlistselectionmanager.cpp <span style="color: grey">(383914df01e9964d60bf009db7af636bd52fd55e)</span></li>
<li>dolphin/src/kitemviews/kitemlistview.h <span style="color: grey">(5723b9aaab26019ecad698f94c9a855ace35766d)</span></li>
<li>dolphin/src/kitemviews/kitemlistview.cpp <span style="color: grey">(72b3fd8fcbfbaa43660fac359320c8227ade9063)</span></li>
<li>dolphin/src/kitemviews/kitemlistviewaccessible.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>dolphin/src/kitemviews/kitemlistviewaccessible.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>dolphin/src/kitemviews/private/kitemlistviewlayouter.h <span style="color: grey">(da5bd1d7d9205ea20bb6112bf3e8ccb01919dae2)</span></li>
<li>dolphin/src/tests/CMakeLists.txt <span style="color: grey">(3f906d18767435080c6b6309ffce5ca2e6445728)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105972/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>