Review Request 127876: use the "selected" state for icons in selected menu items and selected buttons

Hugo Pereira Da Costa hugo.pereira.da.costa at gmail.com
Tue May 10 12:26:47 UTC 2016



> On May 9, 2016, 10:22 p.m., Andreas Kainz wrote:
> > I can't review the code but this is one of the best features in the past 2 years
> > +1 from the vdg
> 
> Hugo Pereira Da Costa wrote:
>     Well, Ship it is a bit premature.
>     I could not test the patch but: it compiles and code looks sensible.
>     However, there are probably pieces missing. I could spot on, on non-flat toolbuttons
>     (should be in bool Style::drawToolButtonLabelControl, round line 4310 or so, for !flat && hasFocus)
>     Also, did you test if it works for readonly comboboxes ? (possibly this is already covered by pushbuttons).
>     You can probably test this with oxyge-demo/oxygen-demo5
>     (with style breeze)
>     
>     Finally, oxygen would need a similar patch. I can put it in, but without being yet able to test (I'd need the other updates). So if you have the time and motivation, feel free :)
> 
> Marco Martin wrote:
>     oxygen-demo5 seems ok for things that depend from the style (the icons in the sidebar would need to be colored as well but i don't think it depends from the style?)
>     
>     oxygen doesn't have menu items or selected buttons colored with highlight colors tough, so probably there aren't parts in oxygen that are affected
> 
> Hugo Pereira Da Costa wrote:
>     Ok even for toolbuttons (see http://wstaw.org/m/2016/05/10/plasma-desktopE24540.png) ?
>     Note that you need to use the Tab key to get the focus on the said buttons. Pressing them will not pass focus to them).
>     
>     For Oxygen: it does have menu item colored, on option! (damn options):
>     http://wstaw.org/m/2016/05/10/plasma-desktopy24540.png
>     but you are right this is not the default, so low priority.
>     And for pushbuttons, you are right.
> 
> Marco Martin wrote:
>     for combo boxes, it will need to reimplement completely drawComboBoxLabelControl as it used to just use the implementation of kstyle

Damn. You are right ...


- Hugo


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


On May 9, 2016, 3:32 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127876/
> -----------------------------------------------------------
> 
> (Updated May 9, 2016, 3:32 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze
> 
> 
> Description
> -------
> 
> with the stylesheet based recoloring for icons, it is possible to make selected icons the same color as the selected text from the palette.
> with the breeze icons style this makes the selected item more readable and more in line with the style.
> 
> 
> Diffs
> -----
> 
>   kstyle/breezestyle.cpp 403771c 
> 
> Diff: https://git.reviewboard.kde.org/r/127876/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> menu.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/05/09/cfbb4571-ed40-411e-ad89-453d29fa2610__menu.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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


More information about the Kde-frameworks-devel mailing list