> On Nov. 19, 2012, 8:07 a.m., Daniele Elmo Domenichelli wrote: > > This is not a real review, just a couple of things that I spotted...
Sure. I imagine this will take a few iterations. Will fix your comments. > On Nov. 19, 2012, 8:07 a.m., Daniele Elmo Domenichelli wrote: > > KTp/Models/abstract-grouping-proxy-model.cpp, line 68 > > <http://git.reviewboard.kde.org/r/107377/diff/1/?file=95310#file95310line68> > > > > qobject_cast? Not a QObject. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107377/#review22203 ----------------------------------------------------------- On Nov. 19, 2012, 6:10 a.m., David Edmundson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107377/ > ----------------------------------------------------------- > > (Updated Nov. 19, 2012, 6:10 a.m.) > > > Review request for Telepathy. > > > Description > ------- > > Initial draft of a replacement for GroupsModel. > The key differences are: > - It doesn't rely on ContactModel* items, just a general model so can be > used with the kpeople model as well as ours > - It can work on a tree (grouping by the top level items in the tree) for > working with the kpeople model > - It can group on any property in the model. Not just GroupName. This will > allow the KPeople model to still group by account. > - As it is generic it can also be used to provide the grouping in the > LogViewer(for example), simplifying the entity model code there. > > The code is pretty complex so I would like people to comment if it makes > sense. > > It is expected one subclasses this model for a specific purpose. i.e to group > by groupNames: > > QSet<QString> GroupGroupingModel::groupsForIndex(const QModelIndex > &sourceIndex) const > { > QStringList groups = > sourceIndex.data(AccountsModel::GroupsRole).value<QStringList>(); > if (groups.isEmpty()) { > groups.append("_unsorted"); > } > > return groups.toSet(); > } > > QVariant GroupGroupingModel::dataForGroup(const QString &group, int role) > const > { > switch (role) { > case Qt::DisplayRole: > if (group == "_unsorted") { > return i18n("Unsorted"); > } else { > return group; > } > case AccountsModel::IdRole: > return group; > } > return QVariant(); > } > > > Also if anyone can think of a better class name for something that group > nodes by group name that isn't GroupGroupingModel that would be much > appreciated. > > > Diffs > ----- > > KTp/Models/abstract-grouping-proxy-model.h PRE-CREATION > KTp/Models/abstract-grouping-proxy-model.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/107377/diff/ > > > Testing > ------- > > See it in action at: [email protected]:scratch/davidedmundson/groupingmodel > > Tested with ContactsModel->FlatModelProxy->This > Tested grouping by GroupName, by account, by client type, and grouping by > presence. > > In the near future ContactsModel will be a list, so this makes more sense. > > No crashes, everything appeared to work. > > > Thanks, > > David Edmundson > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
