> On Jan. 20, 2013, 11:53 a.m., Thomas Pfeiffer wrote:
> > File Attachment: Icon Dialog with 200dpi
> > <http://git.reviewboard.kde.org/r/108504/#fcomment5>
> >
> >     Why is that icon smaller than the others? Is it because the bigger 
> > version of it does not exist?
> 
> Kai Uwe Broulik wrote:
>     Also wondered but just checked, it is only available in max. 48x48

Ok. So from the usability POV, this is definitely an improvement. The code has 
to be checked by "code people" ;) (and IIRC, the usbaility team isn't supposed 
to click "Ship It", to avoid confusion)


- Thomas


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


On Jan. 20, 2013, 11:15 a.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108504/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2013, 11:15 a.m.)
> 
> 
> Review request for kdelibs and KDE Usability.
> 
> 
> Description
> -------
> 
> This makes the KIconDialog (the dialog where you can choose icons for eg. 
> folders) respect the global icon size. Almost all sizes were hardcoded but 
> the patch does away with all of this and works fine with all icon sizes and 
> big font sizes. Also made it aware of FontMetrics (atm with bigger fonts, 
> they also get clipped) and adjusts the grid height accordingly.
> 
> Was fun diving into that "ancient" code :)
> 
> 
> Diffs
> -----
> 
>   kio/kfile/kicondialog.cpp b7d646f 
> 
> Diff: http://git.reviewboard.kde.org/r/108504/diff/
> 
> 
> Testing
> -------
> 
> Yup, see screenshot.
> 
> The only issue that remains is the initial size of the dialog to make it show 
> 4 rows of icons. In the current implementation it just adds another 100px to 
> the dialog height (cf. line 490), which is easy, if all the sizes are known 
> and fixed, but with variing sizes this becomes an issue and I could not think 
> of a proper solution. I probably need to add a sizeHint (tried in the private 
> class, didn't help there)? The easiest but not neccessarily best solution 
> would be to just set the minimumHeight to 4 rows and done.
> 
> 
> File Attachments
> ----------------
> 
> Icon Dialog with 200dpi
>   
> http://git.reviewboard.kde.org/media/uploaded/files/2013/01/20/icondialog.png
> Icon Dialog with 200dpi (without patch)
>   
> http://git.reviewboard.kde.org/media/uploaded/files/2013/01/20/icondialog2.png
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

Reply via email to