> On Oct. 8, 2011, 2:26 p.m., Martin Klapetek wrote:
> > presence-model.cpp, line 90
> > <http://git.reviewboard.kde.org/r/102805/diff/2/?file=38144#file38144line90>
> >
> >     I don't like the bold text on teh 'main' presences. Better solution 
> > would imho be to either leave all the text the same or use separators for 
> > groups, like:
> >     
> >     (O) Available
> >     -------------
> >     (Q) Away
> >     (Q) Went for food
> >     -------------
> >     (O) Busy
> >     (C) Not available

Can I do it after we've merged?

I'll try and build a mockup of it first if that's ok. I'm not 100% sure it will 
look ok, but I'll give it a try.

Alternatively indenting custom items? Needs mockups before code.


> On Oct. 8, 2011, 2:26 p.m., Martin Klapetek wrote:
> > presence-model.cpp, line 143
> > <http://git.reviewboard.kde.org/r/102805/diff/2/?file=38144#file38144line143>
> >
> >     You should check if the presence is not already in the list so we won't 
> > end up with duplicate presences

Agreed.


> On Oct. 8, 2011, 2:26 p.m., Martin Klapetek wrote:
> > kpresence.h, line 28
> > <http://git.reviewboard.kde.org/r/102805/diff/2/?file=38138#file38138line28>
> >
> >     Perhaps this could later be part of the common git repo?

I agree - this class is frickin' awesome. We can do a lot more with it.

Need to sort out the &operator< operator it makes a list each time, which is 
rubbish. 
Then the filter model can use this code, which will be nice.


> On Oct. 8, 2011, 2:26 p.m., Martin Klapetek wrote:
> > presence-model.cpp, line 37
> > <http://git.reviewboard.kde.org/r/102805/diff/2/?file=38144#file38144line37>
> >
> >     I think this should go to the "shared" config ktelepathyrc

Ok.


> On Oct. 8, 2011, 2:26 p.m., Martin Klapetek wrote:
> > main-widget.ui, line 78
> > <http://git.reviewboard.kde.org/r/102805/diff/2/?file=38142#file38142line78>
> >
> >     The combobox should have a maxWidth set, because if the contact list is 
> > opened with very long presence message, the window gets veeeeeeery wide. 
> > Don't forget text eliding. Perhaps the better place for this would be the 
> > GlobalPresenceChooser ctor.
> 
> Rohan Garg wrote:
>     This is actually automatically handled, the max width is set to the max 
> width of the contact list and the presence text is elided accordingly, is 
> there something that I'm missing over here?

I don't understand this either. Ping me on IRC.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102805/#review7178
-----------------------------------------------------------


On Oct. 8, 2011, 1:46 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102805/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2011, 1:46 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> BOOM! Start of the global presence chooser. The UI may need some tweaks 
> later, but it's already many commits long and I'd like to get this merged as 
> soon as possible so everyone can start working on it and sorting out 
> remaining parts together.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 1ff1c6b 
>   abstract-contact-delegate.h 5f73670 
>   abstract-contact-delegate.cpp 34e0d67 
>   dialogs/custom-presence-dialog.h PRE-CREATION 
>   dialogs/custom-presence-dialog.cpp PRE-CREATION 
>   global-presence-chooser.h PRE-CREATION 
>   global-presence-chooser.cpp PRE-CREATION 
>   global-presence.h PRE-CREATION 
>   global-presence.cpp PRE-CREATION 
>   kpresence.h PRE-CREATION 
>   kpresence.cpp PRE-CREATION 
>   main-widget.h b04cbd0 
>   main-widget.cpp 6a734e3 
>   main-widget.ui a84e682 
>   presence-model.h PRE-CREATION 
>   presence-model.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/102805/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

_______________________________________________
KDE-Telepathy mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-telepathy

Reply via email to