> On April 7, 2011, 4:22 p.m., David Edmundson wrote: > > Pretty solid patch. > > > > You don't have anything that updates the image when it changes externally. > > This needs doing (you can make that a future thing, as long as you write it > > down somewhere). > > Does this work when the first account is offline?
It should load the updated avatar as well - see line 286 - this loads the avatar from account and displays it in the menu. It does not update the button icon though and that's because I think when you once set the avatar with that contact list, you intend to keep it (and you have the possibility to select the new one from menu). I think it should work even when the account is offline, because the avatars are cached, so as long as the account is ready, it should work. I think. > On April 7, 2011, 4:22 p.m., David Edmundson wrote: > > main-widget.h, line 71 > > <http://git.reviewboard.kde.org/r/101048/diff/1/?file=13908#file13908line71> > > > > This needs merging with onGenericOperationFinished. (which you don't > > seem to have - update) Yes, I'll merge it. > On April 7, 2011, 4:22 p.m., David Edmundson wrote: > > main-widget.cpp, line 760 > > <http://git.reviewboard.kde.org/r/101048/diff/1/?file=13909#file13909line760> > > > > avatat -> avatar Damn I should turn on a spell checker. > On April 7, 2011, 4:22 p.m., David Edmundson wrote: > > main-widget.cpp, line 823 > > <http://git.reviewboard.kde.org/r/101048/diff/1/?file=13909#file13909line823> > > > > Really you should do some KIO stuff here as KFileDialog can return > > something on the network, but QFile won't open it. > > > > For now I'd be happy with a comment that says it needs doing. True, haven't thought of that. I'll add a FIXME. > On April 7, 2011, 4:22 p.m., David Edmundson wrote: > > main-widget.cpp, line 842 > > <http://git.reviewboard.kde.org/r/101048/diff/1/?file=13909#file13909line842> > > > > You don't close the image if everything processes correctly. > > > > Fortunately a QFile closes itself in the destructor, which means you > > don't actually need to do it all. Uhm...happens :) - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101048/#review2463 ----------------------------------------------------------- On April 7, 2011, 3:33 p.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101048/ > ----------------------------------------------------------- > > (Updated April 7, 2011, 3:33 p.m.) > > > Review request for Telepathy. > > > Summary > ------- > > This patch adds initial avatars support plus KConfig, which holds the > selected avatar. > > > Diffs > ----- > > CMakeLists.txt 548e8c880b5ef921606c7407b92e2e1650faef86 > accounts-model-item.cpp 1d207e7eebb6d286d0fc075dfa4e90628ac92bcc > main-widget.h e310b35d475509f5683a0abe15f21bbb1d8f122e > main-widget.cpp 2e049a13561a1dc4ca0b7117a52ed713d775b9bd > main-widget.ui bc1fb9bf030a927cf635ed1f73d71d6ee33675b2 > > Diff: http://git.reviewboard.kde.org/r/101048/diff > > > Testing > ------- > > Tried to change the avatar and it worked. > > > Screenshots > ----------- > > Avatars menu > http://git.reviewboard.kde.org/r/101048/s/114/ > > > Thanks, > > Martin > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
