> On Aug. 13, 2012, 2:50 p.m., Frank Reininghaus wrote:
> > dolphin/src/kitemviews/kitemlistviewaccessible.h, line 16
> > <http://git.reviewboard.kde.org/r/105972/diff/2/?file=77302#file77302line16>
> >
> >     On the other hand, I see lots of QModelIndex and friends here, which is 
> > something I don't particularly like. After all, the basic idea of the new 
> > view engine is to get rid of Qt's itemviews.
> >     
> >     Is there an easy way to do it without Qt's itemviews?

This is a very good point. For Qt 4 there is no way to avoid QModelIndex since 
it's in the public API. I will look into this for Qt 5 since it's definitively 
something we don't want to rely on.


> On Aug. 13, 2012, 2:50 p.m., Frank Reininghaus wrote:
> > dolphin/src/kitemviews/kitemlistviewaccessible.cpp, line 67
> > <http://git.reviewboard.kde.org/r/105972/diff/2/?file=77303#file77303line67>
> >
> >     I think this will fail when grouping is enabled.

Frank, maybe you can give some guidance here. I didn't have time to carefully 
read the KItemListView etc code, but for me the row/column count/positions etc 
were a bit off. I wonder if there is a general problem in the code or maybe 
just an off-by-one somewhere.


> On Aug. 13, 2012, 2:50 p.m., Frank Reininghaus wrote:
> > dolphin/src/kitemviews/kitemlistviewaccessible.cpp, line 197
> > <http://git.reviewboard.kde.org/r/105972/diff/2/?file=77303#file77303line197>
> >
> >     I think it's preferred to use i18n() and friends for translatable 
> > strings, see http://techbase.kde.org/Development/Tutorials/Localization/i18n

Yes, it should be i18n and I don't think this needs to be there at all. (In 
case of tr using QObject would also be improper, since the class name gets 
added as context, but that doesn't matter since it shouldn't use tr in the 
first place.)


- Frederik


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


On Aug. 13, 2012, 5:50 a.m., Amandeep Singh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105972/
> -----------------------------------------------------------
> 
> (Updated Aug. 13, 2012, 5:50 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 5c1a6da 
>   dolphin/src/kitemviews/kitemlistcontainer.cpp 5500851 
>   dolphin/src/kitemviews/kitemlistcontroller.cpp 88f5d9f 
>   dolphin/src/kitemviews/kitemlistselectionmanager.cpp 383914d 
>   dolphin/src/kitemviews/kitemlistview.h 5723b9a 
>   dolphin/src/kitemviews/kitemlistview.cpp 72b3fd8 
>   dolphin/src/kitemviews/kitemlistviewaccessible.h PRE-CREATION 
>   dolphin/src/kitemviews/kitemlistviewaccessible.cpp PRE-CREATION 
>   dolphin/src/kitemviews/private/kitemlistviewlayouter.h da5bd1d 
>   dolphin/src/tests/CMakeLists.txt 3f906d1 
> 
> Diff: http://git.reviewboard.kde.org/r/105972/diff/
> 
> 
> Testing
> -------
> 
> Focus-tracking tested with KMag / KWin. 
> 
> 
> Thanks,
> 
> Amandeep Singh
> 
>

Reply via email to