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

[Thomas Lübking]

> On Sept. 9, 2014, 8:02 nachm., 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.

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.


- Thomas


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


On Sept. 9, 2014, 8:10 nachm., 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 nachm.)
> 
> 
> 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/20140921/37d17ade/attachment.htm>