> On Dec. 3, 2013, 7:23 p.m., David Edmundson wrote: > > app/emoticon-text-edit-selector.cpp, line 104 > > <https://git.reviewboard.kde.org/r/114275/diff/1/?file=222359#file222359line104> > > > > Could we just replace this first for loop here with: > > > > Q_FOREACH(const QString &emoticonPath, qSort(list.keys())) { > > new EmoticonItem(list[emoticonPath].first(), emoticonPath, > > d->listEmoticon); > > } > > > > rather than looping through everything twice? > > > > Might be easier to read. > > > > (note I've not tested this)
Just so you know, I'm expecting a reply :) Either: "yes, that looks like a good solution which solves the problem" or "No, my original patch is better because of X" - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114275/#review45050 ----------------------------------------------------------- On Dec. 3, 2013, 3:37 p.m., Giuseppe Calà wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/114275/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2013, 3:37 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-text-ui > > > Description > ------- > > First of all, two assumptions: > 1. I know that this code should be supplied upstream to kdelibs but > ktp-text-ui's code is smaller and easier, for me, to manage. > 2. This patch tries to fix what in my opinion is an usability issue. > > Ktp-text-ui uses a QHash from KEmoticonsTheme::emoticonsMap() to build > emoticons' QListWidget. The problem is that items in a QHash are arbitrarily > ordered so the resulting view is messed up. With this patch the returned > QHash is instead used to build a QMap, whose elements are ordered by key, and > use this one as source for EmoticonTextEditItem. The final result is a view > where emoticons are ordered by their filename. > > > Diffs > ----- > > app/emoticon-text-edit-selector.cpp 1fad99b > > Diff: https://git.reviewboard.kde.org/r/114275/diff/ > > > Testing > ------- > > Tested with Google Hangout Emojis Complete theme from kde-look.org: > > Before: http://wstaw.org/m/2013/12/03/emoticons-view-before.png > After: http://wstaw.org/m/2013/12/03/emoticons-view-after.png > > > Thanks, > > Giuseppe Calà > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
