romangg added a comment.

  In D22468#507819 <https://phabricator.kde.org/D22468#507819>, @ngraham wrote:
  
  > 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:
  >
  > 1. 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.
  > 2. The visualization at the top of the KCM with the white background needs 
a visible frame around it. Just give the `Rectangle` in `Screen.qml` an 
appropriate border color and radius.
  > 3. 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)
  > 4. Need to add labels below the slider and also a label to the right that 
shows its current value like this: ``` Global Scale:  |--------------o--------| 
1.6x 1x         1.5x        2x ```
  > 5. You could save some vertical space in the visualization by putting the 
Identify button within each output in the visualization and putting the Center 
View button in the corner and making it only appear on hover (on the desktop at 
least)
  > 6. 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.
  
  
  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.

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D22468

To: romangg, #plasma, #kwin, #vdg, ngraham
Cc: #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

Reply via email to