<table><tr><td style="">andykluger added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D14389">View Revision</a></tr></table><br /><div><div><p>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.</p>
<p>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.</p>
<p>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</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">git format-patch master --stdout</pre></div>
<p>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:</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="diff" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">From 3fc62c74a8feb6e336daef0cd35325808eb2aa7c Mon Sep 17 00:00:00 2001
From: Andy Kluger <andykluger@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
<span style="color: #a00000">---</span>
kdecoration/breezebutton.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
<span style="color: #000080">diff --git a/kdecoration/breezebutton.cpp b/kdecoration/breezebutton.cpp</span>
<span style="color: #000080">index e2e043bc..e6bf2615 100644</span>
<span style="color: #a00000">--- a/kdecoration/breezebutton.cpp</span>
<span style="color: #00a000">+++ b/kdecoration/breezebutton.cpp</span>
<span style="color: #800080">@@ -368,7 +368,7 @@ namespace Breeze</span>
return d->titleBarColor();
<span style="color: #a00000">- } else if( ( type() == DecorationButtonType::KeepBelow || type() == DecorationButtonType::KeepAbove ) && isChecked() ) {</span>
<span style="color: #00a000">+ } else if( ( type() == DecorationButtonType::KeepBelow || type() == DecorationButtonType::KeepAbove || type() == DecorationButtonType::Shade ) && isChecked() ) {</span>
return d->titleBarColor();
<span style="color: #800080">@@ -404,7 +404,7 @@ namespace Breeze</span>
if( type() == DecorationButtonType::Close ) return c->color( ColorGroup::Warning, ColorRole::Foreground );
else return KColorUtils::mix( d->titleBarColor(), d->fontColor(), 0.3 );
<span style="color: #a00000">- } else if( ( type() == DecorationButtonType::KeepBelow || type() == DecorationButtonType::KeepAbove ) && isChecked() ) {</span>
<span style="color: #00a000">+ } else if( ( type() == DecorationButtonType::KeepBelow || type() == DecorationButtonType::KeepAbove || type() == DecorationButtonType::Shade ) && isChecked() ) {</span>
return d->fontColor();
<span style="color: #a00000">-- </span>
2.18.0
From 008b5bff56036365e5c5be74dad481ced745571c Mon Sep 17 00:00:00 2001
From: Andy Kluger <andykluger@gmail.com>
Date: Fri, 27 Jul 2018 11:36:32 -0400
Subject: [PATCH 2/2] flip keep-above icon when checked
<span style="color: #a00000">---</span>
kdecoration/breezebutton.cpp | 32 ++++++++++++++++++++++++--------
1 file changed, 24 insertions(+), 8 deletions(-)
<span style="color: #000080">diff --git a/kdecoration/breezebutton.cpp b/kdecoration/breezebutton.cpp</span>
<span style="color: #000080">index e6bf2615..2932b8b7 100644</span>
<span style="color: #a00000">--- a/kdecoration/breezebutton.cpp</span>
<span style="color: #00a000">+++ b/kdecoration/breezebutton.cpp</span>
<span style="color: #800080">@@ -310,15 +310,31 @@ namespace Breeze</span>
case DecorationButtonType::KeepAbove:
{
<span style="color: #a00000">- painter->drawPolyline( QPolygonF()</span>
<span style="color: #a00000">- << QPointF( 4, 9 )</span>
<span style="color: #a00000">- << QPointF( 9, 4 )</span>
<span style="color: #a00000">- << QPointF( 14, 9 ) );</span>
<span style="color: #00a000">+ if (isChecked())</span>
<span style="color: #00a000">+ {</span>
<span style="color: #a00000">- painter->drawPolyline( QPolygonF()</span>
<span style="color: #a00000">- << QPointF( 4, 13 )</span>
<span style="color: #a00000">- << QPointF( 9, 8 )</span>
<span style="color: #a00000">- << QPointF( 14, 13 ) );</span>
<span style="color: #00a000">+ painter->drawPolyline( QPolygonF()</span>
<span style="color: #00a000">+ << QPointF( 4, 4 )</span>
<span style="color: #00a000">+ << QPointF( 9, 9 )</span>
<span style="color: #00a000">+ << QPointF( 14, 4 ) );</span>
<span style="color: #00a000">+</span>
<span style="color: #00a000">+ painter->drawPolyline( QPolygonF()</span>
<span style="color: #00a000">+ << QPointF( 4, 8 )</span>
<span style="color: #00a000">+ << QPointF( 9, 13 )</span>
<span style="color: #00a000">+ << QPointF( 14, 8 ) );</span>
<span style="color: #00a000">+</span>
<span style="color: #00a000">+ } else {</span>
<span style="color: #00a000">+</span>
<span style="color: #00a000">+ painter->drawPolyline( QPolygonF()</span>
<span style="color: #00a000">+ << QPointF( 4, 9 )</span>
<span style="color: #00a000">+ << QPointF( 9, 4 )</span>
<span style="color: #00a000">+ << QPointF( 14, 9 ) );</span>
<span style="color: #00a000">+</span>
<span style="color: #00a000">+ painter->drawPolyline( QPolygonF()</span>
<span style="color: #00a000">+ << QPointF( 4, 13 )</span>
<span style="color: #00a000">+ << QPointF( 9, 8 )</span>
<span style="color: #00a000">+ << QPointF( 14, 13 ) );</span>
<span style="color: #00a000">+ }</span>
break;
}
<span style="color: #a00000">-- </span>
2.18.0</pre></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R31 Breeze</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D14389">https://phabricator.kde.org/D14389</a></div></div><br /><div><strong>To: </strong>andykluger, Breeze, ngraham<br /><strong>Cc: </strong>ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart<br /></div>