> On March 11, 2011, 12:46 a.m., David Edmundson wrote: > > accountfiltermodel.cpp, line 51 > > <http://git.reviewboard.kde.org/r/100838/diff/2/?file=11004#file11004line51> > > > > It seems a bit wrong to reimplement text filtering in something that > > inherits from QSortFilterProxyModel. For someone using this class there are > > two methods to set a text filter. One that works, one that doesn't. > > > > Your reasoning not to made sense > > (default behaviour is recursive so will not show any contacts if the > > account name doesn't match the string) > > > > but maybe we can get round that by calling > > QSortFilterProxyMode::filterAcceptsRow(source_row, source_parent) here, > > where we know it's a contact.
>From my understanding of the docs, you can either use the regexp. filtering >_or_ to reimplement the filterAcceptsRow by yourself. Since we're already >using it for filtering the offline contacts, there's no point in setting the >regexp for filtering (which IMHO calls filterAcceptsRow anyway). So it is all >implemented in filterAcceptsRow. > On March 11, 2011, 12:46 a.m., David Edmundson wrote: > > accountbutton.cpp, line 34 > > <http://git.reviewboard.kde.org/r/100838/diff/2/?file=11002#file11002line34> > > > > This whole class is very weird. > > > > Never write 'magic numbers' i.e 3 or 7 just unexplained throughout the > > code. > > Storing a C array of every possible enum value seems a wrong approach. > > > > I would store the presence status() - the string (which you can use as > > an ID) as the data() field of the QAction. > > > > onlineAction->setData(Presence::Available().status()); > > > > (you might even be able to just store the Tp::Presence object in there > > if it's a Q_METATYPE (try it and see). ) > > > > Then you can use this to create a proper presence from. > > > > Also your presence lists have no profile support - this can be a future > > thing so I'll explain that later. Alright, I fixed this by setting the Tp::Presence as the data() of actions. This works surprisingly well. I'll update the diff soon. > On March 11, 2011, 12:46 a.m., David Edmundson wrote: > > accountbutton.cpp, line 58 > > <http://git.reviewboard.kde.org/r/100838/diff/2/?file=11002#file11002line58> > > > > This is done for you: > > > > from the docs: > > ---- > > Tp::Account::iconName(); > > As a last resort, "im-" + protocolName() will be returned. > > Fixed. > On March 11, 2011, 12:46 a.m., David Edmundson wrote: > > contactdelegate.cpp, line 79 > > <http://git.reviewboard.kde.org/r/100838/diff/2/?file=11020#file11020line79> > > > > hardcoded values here. At the end of this there should be very few > > fixed numbers. Yes, the delegate needs some work regarding the hardcoded values. Both positions and font metrics. Anyone up to it? - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100838/#review1892 ----------------------------------------------------------- On March 11, 2011, 12:27 a.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100838/ > ----------------------------------------------------------- > > (Updated March 11, 2011, 12:27 a.m.) > > > Review request for Telepathy. > > > Summary > ------- > > This is the work that begun in my clone repo and has matured enough now to be > merged back to 'upstream', where the work should continue now. > > > Diffs > ----- > > CMakeLists.txt 2c62ea10556cdba38d1bca0fe63603df97414336 > accountbutton.h PRE-CREATION > accountbutton.cpp PRE-CREATION > accountfiltermodel.h PRE-CREATION > accountfiltermodel.cpp PRE-CREATION > accounts-model-item.h PRE-CREATION > accounts-model-item.cpp PRE-CREATION > accounts-model.h PRE-CREATION > accounts-model.cpp PRE-CREATION > avatars/astronaut.jpg PRE-CREATION > avatars/chess.jpg PRE-CREATION > avatars/coffee.jpg PRE-CREATION > avatars/dice.jpg PRE-CREATION > avatars/fish.jpg PRE-CREATION > avatars/lightning.jpg PRE-CREATION > avatars/soccerball.png PRE-CREATION > config.h.cmake PRE-CREATION > contact-model-item.h PRE-CREATION > contact-model-item.cpp PRE-CREATION > contactdelegate.h PRE-CREATION > contactdelegate.cpp PRE-CREATION > contactdelegateoverlay.h PRE-CREATION > contactdelegateoverlay.cpp PRE-CREATION > contactoverlays.h PRE-CREATION > contactoverlays.cpp PRE-CREATION > contactviewhoverbutton.h PRE-CREATION > contactviewhoverbutton.cpp PRE-CREATION > filterbar.h PRE-CREATION > filterbar.cpp PRE-CREATION > main-widget.h aed6f7c8336321bc8a6ffb3b9af6b1d493f85790 > main-widget.cpp 6146b62eec17b54be63b594200613931673ac5fe > main-widget.ui 5b0d5aaaf3a4e2f49eb030b98b15fcae3a86170c > main.cpp bba0e4175e8853afe603f26ea4707f4974192d0f > ontologies/CMakeLists.txt 3e0bbe8c634908f689dcd360409aeddcc6fc0d23 > tree-node.h PRE-CREATION > tree-node.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/100838/diff > > > Testing > ------- > > We did a lot of thourough testing on #kde-telepathy, also some people emailed > their reports which all has been fixed. > > > Thanks, > > Martin > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
