> On May 11, 2016, 10:52 a.m., David Edmundson wrote:
> > src/kiconloader.cpp, line 840
> > <https://git.reviewboard.kde.org/r/127875/diff/3/?file=464589#file464589line840>
> >
> >     1) For the unselected case, do you think we want Text and Base rather 
> > than Window and WindowText?
> >     for breeze that's(black and white rather than black and greyish)
> >     
> >     2) If so for the selected case background should change to Highlight 
> > when selected
> >     
> >     I don't think HighlightedText is guaranteed to be visible over Window.

should be greyish window background as in plasma svg uses textcolor and 
backgroundcolor that are mapped to the "window" group rather than the "view" 
group, so corresponding to windowtext and window.

changing background to highlight makes sense, yes will do it (note that once 
this is settled i'll need an almost identical patch in plasma::svg to keep same 
behavior)

the icons that are changed, must be made sure (by the breeze qstyle in this 
case) that are painted over an area painted with highlight color
as in breeze highlightedtext is exactly the color of Window, so they would all 
be invisible just on top of window


- Marco


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


On May 10, 2016, 2:15 p.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, 2:15 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 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
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to