D10522: Added vertical separator

[Alex Nemeth]

anemeth abandoned this revision.
anemeth added a comment.


  This change was submitted to supplement D10438 

  Because it was reverted in D10530  this 
change is not needed anymore.

REPOSITORY
  R31 Breeze

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

To: anemeth, ngraham, hpereiradacosta, #breeze, #vdg
Cc: mart, zzag, mmustac, cfeck, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D10522: Added vertical separator

[Marco Martin]

mart added a comment.


  just as a sensation.. it looks kinda weird to me, also more busy without a 
clear advantage. (and i'm the one that usually wants to put separator lines 
everywhere :p)

REPOSITORY
  R31 Breeze

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

To: anemeth, ngraham, hpereiradacosta, #breeze, #vdg
Cc: mart, zzag, mmustac, cfeck, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D10522: Added vertical separator

[Vlad Zagorodniy]

zzag added a comment.


  I think this diff is not actual anymore.

REPOSITORY
  R31 Breeze

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

To: anemeth, ngraham, hpereiradacosta, #breeze, #vdg
Cc: zzag, mmustac, cfeck, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10522: Added vertical separator

[Vlad Zagorodniy]

zzag added a comment.


  Well, that's too Windows, imo.
  
  > (Note that I am not happy with any on the later community accepted commits 
to breeze either: blur serves no purpose either, nor the extra space allocated 
for checkboxes, etc.)
  
  
  
  > This change doesn't seem very popular, and if it's necessary to make a 
previous change work, that calls into question whether that was the right 
change.
  
  There's a lot of time before next release of Plasma, so "the previous change" 
can still be reverted and no one will notice that.
  I should make it clear: **allocating extra space for check boxes doesn't have 
any logic, it's just to make it look/feel right**. Yes, it gains consistency 
with other os and DEs, but it also **wastes space on display**.
  
  I don't understand what's the point of development without trying something 
new. Sometimes, this "something new" can be a little controversial at the 
beginning, like "big shadows in macOS", but later, people embrace it(now, 
Breeze has bigger window decoration shadows, too). And sometimes, this 
"something new" should be deleted.
  
  I guess that change is the last one because:
  
  - most people don't really like it
  - it can cause stepping down of the Breeze maintainer
  
  So, I will send a diff to revert that "the previous change".

REPOSITORY
  R31 Breeze

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

To: anemeth, ngraham, hpereiradacosta, #breeze, #vdg
Cc: zzag, mmustac, cfeck, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10522: Added vertical separator

[Marijo Mustac]

mmustac added a comment.


  I liked Breeze because of its light and easy concept, get rid of too many 
lines, keep only the ones which make sense or are totally necessary, this 
feeling gets lost here for me so that Breeze would slowly increase into a Storm 
(little overstated I know) Even when this change should happen, I would vote 
for an opt-in feature so that the default keeps clean.

REPOSITORY
  R31 Breeze

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

To: anemeth, ngraham, hpereiradacosta, #breeze, #vdg
Cc: mmustac, cfeck, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10522: Added vertical separator

[Nathaniel Graham]

ngraham added a comment.


  Don't worry Hugo, I don't think we're going to turn Breeze into a mutant that 
you don't recognize. :) This change doesn't seem very popular, and if it's 
necessary to make a previous change work, that calls into question whether that 
was the right change.

REPOSITORY
  R31 Breeze

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

To: anemeth, ngraham, hpereiradacosta, #breeze, #vdg
Cc: cfeck, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10522: Added vertical separator

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  Ok. This time I am strongly against this change. 
  This vertical line serves no purpose, clutters the ui, and is not "simple by 
default".
  Why would one need to separate the checkbox from the icon and text to which 
it is directly related to.
  Also see how it breaks with separator and item selection.
  To me it is a no go.
  (Note that I am not happy with any on the later community accepted commits to 
breeze either: blur serves no purpose either, nor the extra space allocated for 
checkboxes, etc.)
  If people insist on this getting committed, I will oblige, and resign from 
maintaining breeze at the same time, for the reason that it is going in a 
direction which I do not like. I cannot maintain a code which renders to 
something I do not like (nor understand). 
  Sorry
  
  Hugo

REPOSITORY
  R31 Breeze

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

To: anemeth, ngraham, hpereiradacosta, #breeze, #vdg
Cc: cfeck, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10522: Added vertical separator

[Christoph Feck]

cfeck added a comment.


  When I had designed the menus for the Skulpture style, I opted for  not using 
a separate column for checkboxes, but indent the items that use them. The 
separate column was used for icons, though. See 
http://skulpture.maxiom.de/images/skulpture-sample-2.png

REPOSITORY
  R31 Breeze

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

To: anemeth, ngraham, hpereiradacosta, #breeze, #vdg
Cc: cfeck, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10522: Added vertical separator

[Alex Nemeth]

anemeth added a comment.


  The best looking solution would be that menu entries that have 
checkbox/radiobutton, do not get an icon and then we can show the icon and the 
checkbox/radiobutton on the left column.
  Like you see in the Windows 7 pic in the test plan.
  Or maybe I'm just used to that look and I'm biased... :)

REPOSITORY
  R31 Breeze

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

To: anemeth, ngraham, hpereiradacosta, #breeze, #vdg
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D10522: Added vertical separator

[Alex Nemeth]

anemeth updated this revision to Diff 27202.
anemeth added a comment.


  Reduced the width of the horizontal line so it doesn't intersect with the 
vertical line.

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10522?vs=27201&id=27202

BRANCH
  master

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

AFFECTED FILES
  kstyle/breezestyle.cpp

To: anemeth, ngraham, hpereiradacosta, #breeze, #vdg
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D10522: Added vertical separator

[Nathaniel Graham]

ngraham added a comment.


  Reminds me of Windows. I think I could get used to the vertical line, but I 
don't like how the horizontal lines intersect with it. Makes the menu look like 
a grid.

REPOSITORY
  R31 Breeze

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

To: anemeth, ngraham, hpereiradacosta, #breeze, #vdg
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D10522: Added vertical separator

[Alex Nemeth]

anemeth edited the summary of this revision.
anemeth edited the test plan for this revision.
anemeth added reviewers: ngraham, hpereiradacosta, Breeze, VDG.

REPOSITORY
  R31 Breeze

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

To: anemeth, ngraham, hpereiradacosta, #breeze, #vdg
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D10522: Added vertical separator

[Alex Nemeth]

anemeth created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
anemeth requested review of this revision.

REPOSITORY
  R31 Breeze

BRANCH
  master

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

AFFECTED FILES
  kstyle/breezestyle.cpp

To: anemeth
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart