Review Request 127875: Selected state concept for icons

David Edmundson david at davidedmundson.co.uk
Tue May 10 12:37:49 UTC 2016



> On May 9, 2016, 10:32 p.m., David Edmundson wrote:
> > src/kiconloader.cpp, line 841
> > <https://git.reviewboard.kde.org/r/127875/diff/1/?file=464536#file464536line841>
> >
> >     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?
> 
> Marco Martin wrote:
>     the concept is that the svg has parts that use "textcolor" as their text.
>     the monochrome icons would be mostly of text color, mostly black (they can use the other named colors as well if they want).
>     what this patch does is replacing the text color with highlighted text, so the icon goes from black to white.
>     
>     the svg can't have this conditional case "if selected" so all we can do is replacing colors via c++

>the monochrome icons would be mostly of text color, mostly black (they can use the other named colors as well if they want).

So if an icon currently uses textColor and highlightColor it will get screwed with this patch as two previously distinct colours will be the same.


- David


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


On May 10, 2016, 10:56 a.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127875/
> -----------------------------------------------------------
> 
> (Updated May 10, 2016, 10:56 a.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 01d0a8d 
> 
> 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/plasma-devel/attachments/20160510/ac34632e/attachment.html>


More information about the Plasma-devel mailing list