<table><tr><td style="">iasensio 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/D28152">View Revision</a></tr></table><br /><div><div><p>First of all, thanks <a href="https://phabricator.kde.org/p/broulik/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@broulik</a> for such a thorough review and feedback!</p>
<p>About some of your many UI comments, I've already implemented some, while I'm torn on some others, but will leave it to you VDG people decide.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">Do rules have a certain priority order, I don't really see why one would need to re-arrange them? Though the old KCM also had that, so I guess there's a reason to it</li>
</ul></blockquote>
<p>AFAIK, the order affects when a window can be affected by a more general and a more specific rule, but I also don't know the details</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">I think the list view should have an edit button? I hovered an item and instinctively clicked on the "export" button, thinking it would edit the rule, but one can just click the entire delegate. I found this confusing.</li>
</ul></blockquote>
<p>I had had doubts about the edit button vs clicking, but agree:<br />
<a href="https://phabricator.kde.org/F8191084" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">F8191084: Screenshot_20200321_131408.png</a></p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">Generally we use regular buttons rather than tool buttons below the view for "New"</li>
</ul></blockquote>
<p>Here I feel that the non-flat buttons can give a more "busy" feeling when close to the KCM buttons, while flat ones make it more clean, but I'm not strong against it:<br />
<a href="https://phabricator.kde.org/F8191090" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">F8191090: Screenshot_20200321_130927.png</a><br />
<a href="https://phabricator.kde.org/F8191091" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">F8191091: Screenshot_20200321_132006.png</a></p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">The "detect window properties" button is somewhat giant</li>
</ul></blockquote>
<p>Agree.<br />
<a href="https://phabricator.kde.org/F8191093" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">F8191093: Screenshot_20200321_125202.png</a></p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">We generally don't use switches in UI</li>
</ul></blockquote>
<p>I know. In this case I think a checkbox is out of the question visually and semantically, so an alternative can be the yes/no radio buttons like in the old KCM. I still slightly prefer the switch, but the radios doesn't look that bad either.<br />
<a href="https://phabricator.kde.org/F8191103" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">F8191103: Screenshot_20200321_125352.png</a><br />
<a href="https://phabricator.kde.org/F8191104" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">F8191104: Screenshot_20200321_125917.png</a></p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>(Sorry I only came halfway through with code review now)</p></blockquote>
<p>No rush, lucky me, since now there is no place where I can run and hide 😄</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R108 KWin</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D28152">https://phabricator.kde.org/D28152</a></div></div><br /><div><strong>To: </strong>iasensio, Plasma, KWin, VDG<br /><strong>Cc: </strong>hchain, broulik, zzag, kwin, dmenig, manueljlin, Orage, cacarry, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, Ghost6, jraleigh, zachus, fbampaloukas, squeakypancakes, alexde, IohannesPetros, GB_2, mkulinski, trickyricky26, ragreen, jackyalcine, iodelay, crozbo, ndavis, bwowk, ZrenBot, firef, ngraham, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, hardening, romangg, jensreuterberg, aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart<br /></div>