-----------------------------------------------------------
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
>
>