<table><tr><td style="">romangg 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/D22468">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
          color: #6B748C;
          font-style: italic;
          margin: 4px 0 12px 0;
          padding: 8px 12px;
          background-color: #F8F9FC;">
<div style="font-style: normal;
          padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D22468#507819" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;">D22468#507819</a>, <a href="https://phabricator.kde.org/p/ngraham/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@ngraham</a> wrote:</div>
<div style="margin: 0;
          padding: 0;
          border: 0;
          color: rgb(107, 116, 140);"><p>Thanks Roman! This is really excellent overall. A big improvement over the current one . Nevertheless I think we can make it even better and I have some UI review comments:</p>

<ol class="remarkup-list">
<li class="remarkup-list-item">The "Output Settings" section header should contain the name of the output ("Settings for <name>") and then you can remove the output name that's in the FormLayout.</li>
<li class="remarkup-list-item">The visualization at the top of the KCM with the white background needs a visible frame around it. Just give the <tt style="background: #ebebeb; font-size: 13px;">Rectangle</tt> in <tt style="background: #ebebeb; font-size: 13px;">Screen.qml</tt> an appropriate border color and radius.</li>
<li class="remarkup-list-item">The padding between the refresh rate combo box and the horizontal spacer is way too high (and it's not consistent in the amount of spacing above and below the spacer)</li>
<li class="remarkup-list-item">Need to add labels below the slider and also a label to the right that shows its current value like this: <tt style="background: #ebebeb; font-size: 13px;">`</tt> Global Scale:  |--------------o--------| 1.6x 1x         1.5x        2x <tt style="background: #ebebeb; font-size: 13px;">`</tt></li>
<li class="remarkup-list-item">You could save some vertical space in the visualization by putting the <span><span class="phui-tag-view phui-tag-type-shade phui-tag-grey phui-tag-shade "><span class="phui-tag-core ">Identify</span></span></span> button within each output in the visualization and putting the <span><span class="phui-tag-view phui-tag-type-shade phui-tag-grey phui-tag-shade "><span class="phui-tag-core ">Center View</span></span></span> button in the corner and making it only appear on hover (on the desktop at least)</li>
<li class="remarkup-list-item">For that matter, you can hide the visualization  entirely when there's only a single output. That will make the display much simpler and easier to parse for the common case of one screen.</li>
</ol></div>
</blockquote>

<p>I currently concentrate on libkscreen backend work and output duplication and have based other patches on this rewrite here. So I only want to touch this diff again in case of critical issues but not add more detail work on top of what's already there. Instead after merge of the rewrite please create a task with your list of ideas so we can go with multiple small patches afterwards. Also some of these points should be discussed some more and a task is better suited for that.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R104 KScreen</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D22468">https://phabricator.kde.org/D22468</a></div></div><br /><div><strong>To: </strong>romangg, Plasma, KWin, VDG, ngraham<br /><strong>Cc: </strong>VDG, GB_2, gvarsanyi, davidedmundson, ngraham, mvourlakos, knambiar, broulik, filipf, plasma-devel, alexandermilos, LeGast00n, cblack, konkinartem, ian, jguidon, hannahk, Ghost6, jraleigh, fbampaloukas, squeakypancakes, alexde, IohannesPetros, trickyricky26, ragreen, mglb, Pitel, crozbo, ndavis, ZrenBot, firef, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, sebas, apol, mbohlender, mart<br /></div>