Review Request 113969: Do not assume every items have the same height

[David Faure]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/113969/#review55062
-----------------------------------------------------------


I don't know this code so unfortunately I can't really approve it. But since nobody seems to have objections, it sounds like it should go in...

Is this something that can be unit tested?


kdeui/itemviews/kcategorizedview.cpp
<https://git.reviewboard.kde.org/r/113969/#comment38404>

    Coding style comment: some code paths use "break" (which results in return QModelIndex() at the end) and some other code paths (e.g. line 776) do "return QModelIndex()" directly. Any reason for the inconsistency?


- David Faure


On Nov. 20, 2013, 9:47 p.m., Yichao Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/113969/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2013, 9:47 p.m.)
> 
> 
> Review request for kdelibs, David Faure, Rafael Fernández López, and Michael Pyne.
> 
> 
> Bugs: 309780
>     http://bugs.kde.org/show_bug.cgi?id=309780
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> 1. the offset addjust in `KCategorizedView::indexAt` was a no-op (it operates on a temporary variable and is not needed).
> 2. KCategorizedView::indexAt (effectively) assumes all items has the same height when doing bsearch and therefore failed to handle some cases when the text takes multiple lines as shown in the bug report.
> 
> This patch removes the no-op and add special check for items in the same row on the left (or on the right for RightToLeft layout) in order to determine which way the bsearch should go.
> 
> 
> Diffs
> -----
> 
>   kdeui/itemviews/kcategorizedview.cpp 010bcbc 
> 
> Diff: https://git.reviewboard.kde.org/r/113969/diff/
> 
> 
> Testing
> -------
> 
> Compiles and fixes the problem.
> Tested with systemsettings in the following conditions:
> 1. single row in each category.
> 2. multiple rows in each category.
> 3. scrollbar not at the top.
> 
> 
> Thanks,
> 
> Yichao Yu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20140406/01429dad/attachment.htm>