D14389: Invert shade button by same logic as keep-above button

[Andy Kluger]

andykluger added a comment.


  I see how the shade button flips while the keep-above doesn't, but I don't see the difference between toggling the shade state and shade/unshade, or toggling the keep-above state and keep-above/don't-keep-above. But I can't argue against consistency here.
  
  The keep-above button tooltip already changes to "Don't keep above" when it's "checked" so I think it would be better to also flip the keep-above icon rather than stop flipping the shade icon.
  
  I've made the changes in my branch but am new to Phabricator and could use some guidance. I created a new patch set with
  
    git format-patch master --stdout
  
  I clicked "Update Diff" on this Differential page and pasted it. But on the review-the-diff screen it only shows the first change, not both, which are in the patch. I'd like to submit this properly, but for now I'll include it here:
  
    From 3fc62c74a8feb6e336daef0cd35325808eb2aa7c Mon Sep 17 00:00:00 2001
    From: Andy Kluger <andykluger at gmail.com>
    Date: Thu, 26 Jul 2018 00:04:57 -0400
    Subject: [PATCH 1/2] invert colors for shade button whenever already doing so
     for keep-above
    
    ---
     kdecoration/breezebutton.cpp | 4 ++--
     1 file changed, 2 insertions(+), 2 deletions(-)
    
    diff --git a/kdecoration/breezebutton.cpp b/kdecoration/breezebutton.cpp
    index e2e043bc..e6bf2615 100644
    --- a/kdecoration/breezebutton.cpp
    +++ b/kdecoration/breezebutton.cpp
    @@ -368,7 +368,7 @@ namespace Breeze
     
                 return d->titleBarColor();
     
    -        } else if( ( type() == DecorationButtonType::KeepBelow || type() == DecorationButtonType::KeepAbove ) && isChecked() ) {
    +        } else if( ( type() == DecorationButtonType::KeepBelow || type() == DecorationButtonType::KeepAbove || type() == DecorationButtonType::Shade ) && isChecked() ) {
     
                 return d->titleBarColor();
     
    @@ -404,7 +404,7 @@ namespace Breeze
                 if( type() == DecorationButtonType::Close ) return c->color( ColorGroup::Warning, ColorRole::Foreground );
                 else return KColorUtils::mix( d->titleBarColor(), d->fontColor(), 0.3 );
     
    -        } else if( ( type() == DecorationButtonType::KeepBelow || type() == DecorationButtonType::KeepAbove ) && isChecked() ) {
    +        } else if( ( type() == DecorationButtonType::KeepBelow || type() == DecorationButtonType::KeepAbove || type() == DecorationButtonType::Shade ) && isChecked() ) {
     
                 return d->fontColor();
     
    -- 
    2.18.0
    
    
    From 008b5bff56036365e5c5be74dad481ced745571c Mon Sep 17 00:00:00 2001
    From: Andy Kluger <andykluger at gmail.com>
    Date: Fri, 27 Jul 2018 11:36:32 -0400
    Subject: [PATCH 2/2] flip keep-above icon when checked
    
    ---
     kdecoration/breezebutton.cpp | 32 ++++++++++++++++++++++++--------
     1 file changed, 24 insertions(+), 8 deletions(-)
    
    diff --git a/kdecoration/breezebutton.cpp b/kdecoration/breezebutton.cpp
    index e6bf2615..2932b8b7 100644
    --- a/kdecoration/breezebutton.cpp
    +++ b/kdecoration/breezebutton.cpp
    @@ -310,15 +310,31 @@ namespace Breeze
     
                     case DecorationButtonType::KeepAbove:
                     {
    -                    painter->drawPolyline( QPolygonF()
    -                        << QPointF( 4, 9 )
    -                        << QPointF( 9, 4 )
    -                        << QPointF( 14, 9 ) );
    +                    if (isChecked())
    +                    {
     
    -                    painter->drawPolyline( QPolygonF()
    -                        << QPointF( 4, 13 )
    -                        << QPointF( 9, 8 )
    -                        << QPointF( 14, 13 ) );
    +                        painter->drawPolyline( QPolygonF()
    +                            << QPointF( 4, 4 )
    +                            << QPointF( 9, 9 )
    +                            << QPointF( 14, 4 ) );
    +
    +                        painter->drawPolyline( QPolygonF()
    +                            << QPointF( 4, 8 )
    +                            << QPointF( 9, 13 )
    +                            << QPointF( 14, 8 ) );
    +
    +                    } else {
    +
    +                        painter->drawPolyline( QPolygonF()
    +                            << QPointF( 4, 9 )
    +                            << QPointF( 9, 4 )
    +                            << QPointF( 14, 9 ) );
    +
    +                        painter->drawPolyline( QPolygonF()
    +                            << QPointF( 4, 13 )
    +                            << QPointF( 9, 8 )
    +                            << QPointF( 14, 13 ) );
    +                    }
                         break;
                     }
     
    -- 
    2.18.0

REPOSITORY
  R31 Breeze

REVISION DETAIL
  https://phabricator.kde.org/D14389

To: andykluger, #breeze, ngraham
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180727/efc5a42c/attachment-0001.html>