Review Request: Fix some UI bugs with high resolution screens

Michael Zanetti mzanetti at kde.org
Sun Oct 21 12:09:54 UTC 2012



> On Oct. 21, 2012, 12:36 a.m., Lamarque Vieira Souza wrote:
> > applet/activatableitem.cpp, line 51
> > <http://git.reviewboard.kde.org/r/106961/diff/1/?file=91205#file91205line51>
> >
> >     Is there any need to change this? 4 is the default spacing in Q*Layout classes.


> On Oct. 21, 2012, 12:36 a.m., Lamarque Vieira Souza wrote:
> > applet/nmpopup.cpp, line 105
> > <http://git.reviewboard.kde.org/r/106961/diff/1/?file=91208#file91208line105>
> >
> >     You could make all those QFontMetrics(...).height() calls run just once and use just the result, like I did for rowHeight.

This depends on the font used. For example this one (m_leftLabel) uses a bigger font than the rest so it needs to be calculated here. However, I re-used the maximumHeight now to set the minimumHeight to calculate it only once at least.


> On Oct. 21, 2012, 12:36 a.m., Lamarque Vieira Souza wrote:
> > applet/wirelessnetworkitem.cpp, line 125
> > <http://git.reviewboard.kde.org/r/106961/diff/1/?file=91210#file91210line125>
> >
> >     If the result is the same I rather prefer not to use float point operations  here and the lines below.

Ok. I've attached the height of the strength meter to the buttons font height now. It looks pretty much the same as before, scales nicely with the DPI and eliminates the floating point operations. In the lines below I realized that its again only the spacer where the height doesn't matter and reverted it back to 12. Also I've found a duplicate setMaximumHeight() and removed it.


> On Oct. 21, 2012, 12:36 a.m., Lamarque Vieira Souza wrote:
> > applet/activatableitem.cpp, line 43
> > <http://git.reviewboard.kde.org/r/106961/diff/1/?file=91205#file91205line43>
> >
> >     Why did you change this? A 2 pixels icon is too small to be usefull.

leftover from testing which i missed... Adapts to DesktopIcon's size now which is 48 by default.


- Michael


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


On Oct. 21, 2012, 12:09 p.m., Michael Zanetti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106961/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2012, 12:09 p.m.)
> 
> 
> Review request for Network Management.
> 
> 
> Description
> -------
> 
> Some more fixes for Retina screen. Because the font scales with the DPI it doesn't fit any more on the hardcoded button sizes. Here are example screenshots:
> 
> http://notyetthere.org/data/kde/nm-fixed-sizes.png
> http://notyetthere.org/data/kde/nm-dynamic-sizes.png
> 
> 
> Diffs
> -----
> 
>   applet/activatableitem.cpp 1198fd2 
>   applet/interfacedetailswidget.cpp 9635559 
>   applet/interfaceitem.cpp 356c285 
>   applet/nmpopup.cpp a8dfd54 
>   applet/wirelessinterfaceitem.cpp 8f71b20 
>   applet/wirelessnetworkitem.cpp 6489f5d 
> 
> Diff: http://git.reviewboard.kde.org/r/106961/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Zanetti
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20121021/cce1b49a/attachment.html>


More information about the kde-networkmanager mailing list