<div class="gmail_quote">2011/3/10 David Edmundson <span dir="ltr"><<a href="mailto:david@davidedmundson.co.uk">david@davidedmundson.co.uk</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
This is really just for people working on the contact list.<div><br></div><div>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.</div>
<div><br></div><div>1) Coding Style</div><div><br>We follow this</div><div><a href="http://techbase.kde.org/Policies/Kdelibs_Coding_Style" target="_blank">http://techbase.kde.org/Policies/Kdelibs_Coding_Style</a></div>
<div><br></div><div>In particular </div><div>if (something) { <- brace on the end of the line. Always put the brace, even if it's for 1 line.</div><div><br></div><div>}</div></blockquote><div><br></div><div>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)</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div><br></div><div>--------</div><div>
<br>
</div><div>2) Don't hardcode fonts and colours in the delegate code.</div><div><br></div><div>If you write font.setSize(10); that doesn't work if someone has vision issues or even just owns a massive resolution monitor. </div>
<div><br></div><div>Use KGlobalSettings::smallestReadableFont() </div><div> - and its equivalents for other fonts.</div></blockquote><div><br></div><div>Ok.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div><br></div><div>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.</div></blockquote><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div><br></div><div>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.</div>
<div>QPalette should have everything you need.</div></blockquote><div><br></div><div>Ah, I didn't thought about that. Very good point, thanks. I'll work on this as well.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div><br></div><div>---------</div><div><br></div><div>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.</div>
<div><br></div><div>I'm not sure what a better approach is, but there must be one.</div><div><br></div></blockquote><div><br></div><div>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. </div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div></div><div>--------</div><div><br></div><div>Overall it's still pretty awesome, hopefully this will speed up the process of having it merged into the real master.</div>
</blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div><br></div><div>I'm happy to help with any of these.</div></blockquote><div><br></div><div><meta http-equiv="content-type" content="text/html; charset=utf-8">You could do something about those fonts and the sizeHint() ;)</div>
<div> </div><div><br></div><div>Marty</div></div><br>