Review Request 127875: Selected state concept for icons

[David Edmundson]

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




src/kiconloader.cpp (line 311)
<https://git.reviewboard.kde.org/r/127875/#comment64633>

    was createIconImage in the last frameworks?
    
    if so this is an ABI break.
    
    instead of using a default argument you'll have to have two methods.



src/kiconloader.cpp (line 820)
<https://git.reviewboard.kde.org/r/127875/#comment64634>

    these should be QStringLiteral ?



src/kiconloader.cpp (line 841)
<https://git.reviewboard.kde.org/r/127875/#comment64635>

    we're already passing a highlightedText colour for some reason.
    
    Either:
    - Kiconloader is responsible for choosing whether to use the normal colour or the highlight colour
    OR
     - The SVG is
    
    Right now it looks like it's a mixture?


- David Edmundson


On May 9, 2016, 3:26 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127875/
> -----------------------------------------------------------
> 
> (Updated May 9, 2016, 3:26 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> -------
> 
> QIcon has a Selected state that wasn't mapped to KIcon, use it and in case for svg based icons that take colors from the palette take the highlightedText color from the palette to color the icon instead of the text color, making it possible for styles to have white icons and white text in selected menu items (need explicit support from the style, patches in breeze and the like coming)
> 
> 
> Diffs
> -----
> 
>   src/kiconengine.cpp 7c72ade 
>   src/kiconloader.h cf2f58a 
>   src/kiconloader.cpp b3c7166 
> 
> Diff: https://git.reviewboard.kde.org/r/127875/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> menu.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/05/09/0fb9a44c-8db4-4a10-91e7-1a6a36e41f62__menu.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160509/ff90d6f3/attachment.html>