<table><tr><td style="">mart requested changes to this revision.<br />mart 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/D11197">View Revision</a></tr></table><br /><div><div><p>I'm ok with it, tough i would change a bit the logic, as described</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11197#inline-54965">View Inline</a><span style="color: #4b4d51; font-weight: bold;">OverlaySheet.qml:117</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d">     */</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #004012">property</span> <span style="color: #004012">bool</span> <span style="color: #aa4000">showCloseButton:</span> <span style="color: #000a65">true</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">property alias showCloseButton: closeIcon.visible</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11197#inline-54966">View Inline</a><span style="color: #4b4d51; font-weight: bold;">OverlaySheet.qml:304</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; ">            <span style="color: #aa4000">z:</span> <span style="color: #601200">3</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">            <span style="color: #aa4000">visible:</span> <span style="color: #aa2211">!</span><span style="color: #004012">Settings</span><span class="p">.</span><span style="color: #004012">isMobile</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">            <span style="color: #aa4000">visible:</span> <span style="color: #aa2211">!</span><span style="color: #004012">Settings</span><span class="p">.</span><span style="color: #004012">isMobile<span class="bright"></span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">&&</span></span><span class="bright"> </span><span style="color: #004012"><span class="bright">showCloseButton</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; ">            <span style="color: #aa4000">width:</span> <span style="color: #004012">Units</span><span class="p">.</span><span style="color: #004012">iconSizes</span><span class="p">.</span><span style="color: #004012">smallMedium</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">with the alias then  we again have only:<br />
visible: !Settings.isMobile</p>

<p style="padding: 0; margin: 8px;">in this case, the default behavior of showCloseButton will be !Settings.isMobile, and then the developer could always override the behavior, breaking the binding to fore it always true or always false (so the property will work even on mobile)</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R169 Kirigami</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D11197">https://phabricator.kde.org/D11197</a></div></div><br /><div><strong>To: </strong>ngraham, mart, Kirigami<br /><strong>Cc: </strong>plasma-devel, apol, davidedmundson, mart, hein<br /></div>