2011/3/10 David Edmundson <[email protected]> > This is really just for people working on the contact list. > > I've been doing a few minor edits on it recently and there is a lot of code > that really needs tidying up before it can be merged. I thought I would > highlight these here so we can all work on that, and get it fixed soon. > > 1) Coding Style > > We follow this > http://techbase.kde.org/Policies/Kdelibs_Coding_Style > > In particular > if (something) { <- brace on the end of the line. Always put the brace, > even if it's for 1 line. > > } >
Yes, I'm following this now with new code. Though I want to do a major code polishing before merging it to the main tree. So all the code will follow the kde style in the end (and I'n also trying to replace it whenever I see it in the code I'm working with) > > -------- > > 2) Don't hardcode fonts and colours in the delegate code. > > If you write font.setSize(10); that doesn't work if someone has vision > issues or even just owns a massive resolution monitor. > > Use KGlobalSettings::smallestReadableFont() > - and its equivalents for other fonts. > Ok. > > With this sizeHint code needs to be more complex, and use font metric > information to work out the minimum height the box needs to be. > Well the minimum size is determined by the icon size, so as long as we use constat icon size, we can leave the minimum size same as the icon size (+ some padding). Though it makes sense to return bigger sizeHint in case of bigger fonts. I'll try to take care of this. > > Same for colours. If you switch your colour scheme in systemsettings to > "Obsidian Coast" a dark colour scheme, your delegate with the hardcoded > background colour stands out really obviously as being wrong. > QPalette should have everything you need. > Ah, I didn't thought about that. Very good point, thanks. I'll work on this as well. > > --------- > > 3) Looking at whether the alias is blank or not seems a slightly hacky way > to try and separate headings in the model from contacts. > > I'm not sure what a better approach is, but there must be one. > > Well the AliasRole is classified as a contact role, so it *should* be non-empty only if the current item is contact. I have to look at the Tp-Qt4 Yell how they handle this. > -------- > > Overall it's still pretty awesome, hopefully this will speed up the process > of having it merged into the real master. > > I'm happy to help with any of these. > You could do something about those fonts and the sizeHint() ;) Marty
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
