-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105972/#review17283
-----------------------------------------------------------


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.


- Frank Reininghaus


On Aug. 11, 2012, 9:34 a.m., Amandeep Singh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105972/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2012, 9:34 a.m.)
> 
> 
> Review request for Dolphin, KDE Base Apps and KDE Accessibility.
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/CMakeLists.txt 5c1a6dad58cb579ee85731d4bfa0ebe9a6a1bea4 
>   dolphin/src/kitemviews/kitemlistcontainer.cpp 
> 5500851c8c92c564bf3130c66198cea9b61eb8c7 
>   dolphin/src/kitemviews/kitemlistcontroller.cpp 
> 88f5d9f7cff1614fb8595b470d46916584710a27 
>   dolphin/src/kitemviews/kitemlistselectionmanager.cpp 
> 383914df01e9964d60bf009db7af636bd52fd55e 
>   dolphin/src/kitemviews/kitemlistview.h 
> 5723b9aaab26019ecad698f94c9a855ace35766d 
>   dolphin/src/kitemviews/kitemlistview.cpp 
> 72b3fd8fcbfbaa43660fac359320c8227ade9063 
>   dolphin/src/kitemviews/kitemlistviewaccessible.h PRE-CREATION 
>   dolphin/src/kitemviews/kitemlistviewaccessible.cpp PRE-CREATION 
>   dolphin/src/kitemviews/private/kitemlistviewlayouter.h 
> da5bd1d7d9205ea20bb6112bf3e8ccb01919dae2 
>   dolphin/src/tests/CMakeLists.txt 3f906d18767435080c6b6309ffce5ca2e6445728 
> 
> Diff: http://git.reviewboard.kde.org/r/105972/diff/
> 
> 
> Testing
> -------
> 
> Focus-tracking tested with KMag / KWin. 
> 
> 
> Thanks,
> 
> Amandeep Singh
> 
>

Reply via email to