-----------------------------------------------------------
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
> 
>

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to