Review Request 120120: kmenuedit: do not resize app icons (fixes #338883)

[Boris Egorov]

> On Sept. 9, 2014, 8:02 p.m., Thomas Lübking wrote:
> > treeview.cpp, line 232
> > <https://git.reviewboard.kde.org/r/120120/diff/2/?file=310611#file310611line232>
> >
> >     Maybe rather try to limit to the font height instead?
> 
> Christoph Feck wrote:
>     Why? We use "Small" icon size next to text everywhere (buttons, menu items etc), so we expect the user to set a sane size (and the user expects the developer to respect that size).
> 
> Thomas Lübking wrote:
>     Apparently is was fixed to 20px to "somehow align to the font height on my system"
>     
>     Things may look weird if the icon is "slightly" larger than the text (you don't get a text with a large icon as in an iconview, but a small icon and text with "broken" line spacing.
>     Also, the icon may as well end up being much smaller than the text (if the small icon size is kept at 22px, but large text is used for a11y reasons)
> 
> Albert Astals Cid wrote:
>     Boris? Can you address this comments?
> 
> Boris Egorov wrote:
>     So, as I understand, Thomas proposes to limit icon size to customized text size in both directions (min/max). On the other hand, I agree with Christoph that we must respect user settings. We should look how similar settings handled in another KDE apps.
>     I think if we enforce some limitation, we should warn user (at runtime or in documentation at least) that icon size cannot be set to any size.
> 
> Thomas Lübking wrote:
>     The user can set the size to whatever he wants - the question is whether KIconLoader::Small is the preferable icon size *in this particular context* or whether the icon size should follow the text height for a somewhat aligned look.
>     
>     You don't need to warn the user, rather add Jens Reuterberg to this review for an artistical comment.

Thanks for explanation, Thomas.
I think it would be better to use KIconLoader::Small. As for aligning, is it completely impossible to make it like in dolphin? Icon size is changing and it is still aligned correctly in Compact View mode.
http://i.imgur.com/ASZwuYG.png
http://i.imgur.com/VwXzQtX.png


- Boris


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120120/#review66152
-----------------------------------------------------------


On Sept. 9, 2014, 8:10 p.m., Boris Egorov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120120/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2014, 8:10 p.m.)
> 
> 
> Review request for kde-workspace.
> 
> 
> Bugs: 338883
>     https://bugs.kde.org/show_bug.cgi?id=338883
> 
> 
> Repository: kmenuedit
> 
> 
> Description
> -------
> 
> Remove code which restricts app icons to 20x20 pixels
> 
> 
> Diffs
> -----
> 
>   treeview.cpp 99165ca 
> 
> Diff: https://git.reviewboard.kde.org/r/120120/diff/
> 
> 
> Testing
> -------
> 
> Build app, run it.
> 
> 
> Thanks,
> 
> Boris Egorov
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20140923/58d1d7ed/attachment.htm>