<table><tr><td style="">ngraham requested changes to this revision.<br />ngraham added a comment.<br />This revision now requires changes to proceed.
</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>Hmm, the issue here is that unlike the Keep Above button, the Shade button actually changes its icon when used. So it's not so much a "shade enabled/shade disabled" button, as it is a "shade/unshade" button. Do you see the difference? One tracks state ("is shade mode enabled?"), the other one expresses distinct actions ("shade the window!" "un-shade the window!"). I would want that harmonized with the Keep Above button so they both behave in the same way first before accepting this patch, because otherwise people will file bugs saying "The Keep Above button doesn't change its icon when it's turned on the way the Shade button does!"</p></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>