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