D10438: increase left/right margin of menuitems

[Alex Nemeth]

anemeth added a comment.


  @zzag Did you accidentally removed the changes in breeze.h and breezestyle.h? 
:)

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Vlad Zagorodniy]

zzag added a comment.


  How it currently looks:
  
  F5707975: 1.png 
  
  F5707978: 2.png 

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Vlad Zagorodniy]

zzag updated this revision to Diff 27019.
zzag added a comment.


  reserve space for checkable widgets in menu items

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10438?vs=26980&id=27019

BRANCH
  menuitem-margins

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

AFFECTED FILES
  kstyle/breezestyle.cpp

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

D10438: increase left/right margin of menuitems

[Alex Nemeth]

anemeth added a comment.


  In D10438#205042 , @zzag wrote:
  
  > Did you change the left margin? Could you please test the left margin with 
values 20/24/28 and pick the best one?
  
  
  I didn't change the margins previously, only applied the patch. Here are they 
changed now:
  F5707947: 20.PNG 
  20 looks better, but 28 looks centered.
  Since this is with icons + shortcuts disabled it should not be used as 
reference.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  In D10438#205041 , @zzag wrote:
  
  > > But then that makes it a different patch, right ?
  >
  > Why?
  
  
  Because I know the code: I wrote it.
  
  >> Margins are not changed. What is changed is the allocated space for the 
menu item content, that always accommodate for the checkbox, and draws it 
whenever there is one.
  > 
  > No, margins still have to be changed. I would like to have static margins 
but how to add space on the left side if there is no any menu items with a 
checkable widget? Maybe I'm missing something? I should have added the word 
reserved in quotes.
  
  This is incorrect.
  The only thing you need to change to accomodate for enough space for 
checkboxes on all menu items disregarding whether there is a checkbox or not is 
to comment the two calls to:
  
if( menuItemOption->menuHasCheckableItems )
  
  Thats a very small patch that requires no change to the margins.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Vlad Zagorodniy]

zzag added a comment.


  In D10438#205007 , @anemeth wrote:
  
  > F5707729: .PNG 
  
  
  Did you change the left margin? Could you please test the left margin with 
values 20/24/28 and pick the best one?

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Vlad Zagorodniy]

zzag added a comment.


  > But then that makes it a different patch, right ?
  
  Why?
  
  > Margins are not changed. What is changed is the allocated space for the 
menu item content, that always accommodate for the checkbox, and draws it 
whenever there is one.
  
  No, margins still have to be changed. I would like to have static margins but 
how to add space on the left side if there is no any menu items with a 
checkable widget? Maybe I'm missing something? I should have added the word 
reserved in quotes.
  
  > The first part of that tends to make the checkbox be closest to the edge. 
Are we thinking we should/not use icons in menus, keep the same?
  
  What do you mean?

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Nathaniel Graham]

ngraham added a comment.


  In D10438#205010 , @abetts wrote:
  
  > In D10438#205009 , @ngraham 
wrote:
  >
  > > Let's try to avoid radical proposals like getting rid of keyboard 
shortcuts and icons. For now, let's focus on the initial goal, or perhaps a 
slightly modified goal of adding room to put checkboxes to the left of the text 
(which I would support).
  >
  >
  > Can you answer the question though?
  
  
  I would not be in favor of removing the menu item icons, no. This would 
introduce inconsistency issues all over the place and represents a fairly 
significant departure from a long-standing UI. If we ever do that, it would 
have to:
  
  - Present clear usability and visual advantages that enough folks agree on
  - Involve significant discussion of the ramifications
  - Be in in its own patch
  - Involve a great deal of testing to make sure there's no fallout
  - etc
  
  ...In short, not something worth bringing up in the discussion of this patch.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Vlad Zagorodniy]

zzag added a comment.


  In D10438#205009 , @ngraham wrote:
  
  > Let's try to avoid radical proposals like getting rid of keyboard shortcuts 
and icons. For now, let's focus on the initial goal, or perhaps a slightly 
modified goal of adding room to put checkboxes to the left of the text (which I 
would support).
  
  
  I haven't got rid of keyboard shortcuts. I've encountered this bug too. With 
`ShowIconsInMenuItems` set to `false`, `QStyleOptionMenuItem` doesn't pass 
shortcuts.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Andres Betts]

abetts added a comment.


  In D10438#205009 , @ngraham wrote:
  
  > Let's try to avoid radical proposals like getting rid of keyboard shortcuts 
and icons. For now, let's focus on the initial goal, or perhaps a slightly 
modified goal of adding room to put checkboxes to the left of the text (which I 
would support).
  
  
  Can you answer the question though?

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Nathaniel Graham]

ngraham added a comment.


  Let's try to avoid radical proposals like getting rid of keyboard shortcuts 
and icons. For now, let's focus on the initial goal, or perhaps a slightly 
modified goal of adding room to put checkboxes to the left of the text (which I 
would support).

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Alex Nemeth]

anemeth added a comment.


  In D10438#205005 , @abetts wrote:
  
  > Are we thinking we should/not use icons in menus, keep the same?
  
  
  Looks quite good in my opinion with this patch applied and no icons
  F5707729: .PNG 

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Andres Betts]

abetts added a comment.


  How are we accounting for stacking menu items that have:
  
  Checkbox - Item Icon - Label -- Shortcut-Arrow Icon
  
  The first part of that tends to make the checkbox be closest to the edge. Are 
we thinking we should/not use icons in menus, keep the same?

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Nathaniel Graham]

ngraham added a comment.


  We cannot ever remove the shortcuts from the menus, for countless usability 
reasons. Lack of discoverable keyboard shortcuts is one of the many 
productivity and ergonomic flaws of GNOME; we cannot regress to that level.
  
  in macOS, the shortcuts are more compact because symbols are used instead of 
letters:
  
  F5707708: image.png 
  
  I'm not sure we could use that here because PC keyboards' modifier keys don't 
have symbols on them the way Mac keyboards generally do.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  In D10438#204922 , @zzag wrote:
  
  > Just to clarify that other DE and OS have some space on the left side:
  >
  > - Windows: 
https://forums.windowscentral.com/attachments/windows-8/48119d1382981718t-screenshot-1-.png
  > - macOS: https://i.stack.imgur.com/h6LAN.png
  > - elementaryOS: https://i.stack.imgur.com/2fwU9.png
  >
  >   even Ubuntu(Unity):
  >
  >   F5707179: context-menu-ubuntu.png 
  >
  >   F5707182: context-menu-tray.png 
  >
  >   Typical pattern: [] []  [ or ] 
in the case when a menu item doesn't have a checkbox or radiobutton, some space 
is reserved(more precisely, width of checkbox/radiobutton).
  
  
  Agreed, and if others from vdg agree, I am fine with always adding the 
necessary space for checkboxes and radio buttons on the left.
  But then that makes it a different patch, right ?
  
  Margins are not changed. What is changed is the allocated space for the menu 
item content, that always accommodate for the checkbox, and draws it whenever 
there is one.
  
  Personally, I think it is better that this space is not allocated when there 
is no crossheck to be drawn in the full menu. I think it is an _improvement_ 
with respect to the other design you send, and would see this change rather as 
a regression. (I would not see why you allocate some empty space if there is 
nothing to render in it).
  
  But then, if VDG agrees, I would oblige.
  
  > So, the left margin should be even bigger - 28. (CheckBox_Size + 
2*MenuItem_ItemSpacing)

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  In D10438#204960 , @anemeth wrote:
  
  > In D10438#204922 , @zzag wrote:
  >
  > > Just to clarify that other DE and OS have some space on the left side:
  > >
  > > - Windows: 
https://forums.windowscentral.com/attachments/windows-8/48119d1382981718t-screenshot-1-.png
  > > - macOS: https://i.stack.imgur.com/h6LAN.png
  > > - elementaryOS: https://i.stack.imgur.com/2fwU9.png
  >
  >
  > you're right
  >  one more thing I noticed in the screenshots here that there is no keyboard 
shortcut hint that makes the menu double the width it should be
  >
  > F5707644: qq.PNG 
  >
  > maybe in an another patch that could be removed too?
  
  
  That would be a serious regression to many users. How would you then know 
about the shortcuts ?

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Alex Nemeth]

anemeth added a comment.


  In D10438#204922 , @zzag wrote:
  
  > Just to clarify that other DE and OS have some space on the left side:
  >
  > - Windows: 
https://forums.windowscentral.com/attachments/windows-8/48119d1382981718t-screenshot-1-.png
  > - macOS: https://i.stack.imgur.com/h6LAN.png
  > - elementaryOS: https://i.stack.imgur.com/2fwU9.png
  
  
  you're right
  one more thing I noticed in the screenshots here that there is no keyboard 
shortcut hint that makes the menu double the width it should be
  
  F5707644: qq.PNG 
  
  maybe in an another patch that could be removed too?

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Vlad Zagorodniy]

zzag added a comment.


  In D10438#204884 , 
@hpereiradacosta wrote:
  
  > In anycase, increasing margins should then be made consistently accross the 
style and not one by one if you want to keep balance.
  
  
  No, it should not. This is a small change which doesn't require rethinking 
the whole design, imo.
  
  > For gnome, for instance, see how _all_ margins are larger (and thus 
consistent).
  
  GNOME just loves "nom-nom-nom available display space".

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Vlad Zagorodniy]

zzag added a comment.


  Just to clarify that other DE and OS have some space on the left side:
  
  - Windows: 
https://forums.windowscentral.com/attachments/windows-8/48119d1382981718t-screenshot-1-.png
  - macOS: https://i.stack.imgur.com/h6LAN.png
  - elementaryOS: https://i.stack.imgur.com/2fwU9.png
  
  even Ubuntu(Unity):
  
  F5707179: context-menu-ubuntu.png 
  
  F5707182: context-menu-tray.png 
  
  Typical pattern:
  [] []  [ or ]
  in the case when a menu item doesn't have a checkbox or radiobutton, some 
space is reserved(more precisely, width of checkbox/radiobutton).
  
  So, the left margin should be even bigger - 28. (CheckBox_Size + 
2*MenuItem_ItemSpacing)
  
  ---
  
  > According to the HIG units.smallSpacing (4px) or units.largeSpacing (18px) 
or multiples of these should be used when ever possible, maybe it would make 
sense to uses these?
  
  So, margins should be preferred to multiples of 4 or 18, right?
  
  > also see how in two the posted screenshots the checkmarks for checkable 
items is actually very close to the menu border
  
  It can be changed. Currently, distance between menu's left border and 
checkboxes equals to `MenuItem_ItemSpacing`.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  In D10438#204764 , @fabianr wrote:
  
  > According to the HIG  units.smallSpacing (4px) or units.largeSpacing (18px) 
or multiples of these should be used when ever possible, maybe it would make 
sense to uses these?
  
  
  Thats a good point. One should use update the breeze metrics (which predate 
the HIG), to use these whenever possible. 
  This is largely unrelated to this patch though.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  also see how in two the posted screenshots the checkmarks for checkable items 
is actually very close to the menu border.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  In D10438#204869 , @januz wrote:
  
  > IMO, the last version looks better than the current menu. That said, I 
think the top/bottom paddings are still too tight, I would try adding 2-3px for 
each.
  >  It's true that there's a question of taste but more whitespace is 
generally a good thing (unless you go overboard and start making huge widgets). 
A couple more pixels in the menus will help focus the elements better (by 
framing them in negative space), it will make the ui look less "full of stuff" 
and less tense.
  >
  > For reference:
  >
  > Material design manual: 
https://material.io/guidelines/components/menus.html#menus-usage
  
  
  This is a touch based ui.
  
  > Gnome: http://i.imgur.com/er2odvE.png
  >  Mac: 
https://www.intego.com/mac-security-blog/wp-content/uploads/2016/12/Mac-menu-bar-extras-sound.png
  >  Windows: 
https://docs.microsoft.com/en-us/windows/uwp/design/controls-and-patterns/menus
  
  None of these are application's menu. (though I did not check if applications 
menus are narrower in these three cases)
  
  In anycase, increasing margins should then be made consistently accross the 
style and not one by one if you want to keep balance.
  For gnome, for instance, see how _all_ margins are larger (and thus 
consistent).
  
  So far, on bugzilla there have been more complains about breeze being too 
space-hungry than too dense. 
  (I can post the links here if needed)

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Diego Gangl]

januz added a comment.


  IMO, the last version looks better than the current menu. That said, I think 
the top/bottom paddings are still too tight, I would try adding 2-3px for each.
  It's true that there's a question of taste but more whitespace is generally a 
good thing (unless you go overboard and start making huge widgets). A couple 
more pixels in the menus will help focus the elements better (by framing them 
in negative space), it will make the ui look less "full of stuff" and less 
tense.
  
  For reference:
  
  Material design manual: 
https://material.io/guidelines/components/menus.html#menus-usage
  Gnome: http://i.imgur.com/er2odvE.png
  Mac: 
https://www.intego.com/mac-security-blog/wp-content/uploads/2016/12/Mac-menu-bar-extras-sound.png
  Windows: 
https://docs.microsoft.com/en-us/windows/uwp/design/controls-and-patterns/menus

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  In D10438#204757 , @zzag wrote:
  
  > @mmustac what about this?
  >
  > F5706606: without-checkboxes-10.png 
  >
  > margins:
  >
  > - left: 10
  > - right: 4
  > - top,bottom: 3
  >
  >   ---
  >
  > > The new margins shown in the screenshot are just too huge for my taste.
  >
  > Well, it's hard to target each personal taste. For me, bigger left margin 
feels more natural. Typical the "for my taste" problem.
  
  
  Sorry to say but to my taste current margins are fine and new one two large
  Now, to address the taste issue:
  1 What is the use case?
  2 are there bug reports about margins being too narrow?  (From our users?)
  3 is this consistent with other margins used elsewhere in breeze ?
  
  For 2 at leasthe,  bug reports are generally theother way around,  from 
people complaining that breeze wastes too much space
  
  So -1 from my side sorry.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Fabian Riethmayer]

fabianr added a comment.


  According to the HIG  units.smallSpacing (4px) or units.largeSpacing (18px) 
or multiples of these should be used when ever possible, maybe it would make 
sense to uses these?

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Vlad Zagorodniy]

zzag added a comment.


  @mmustac what about this?
  
  F5706606: without-checkboxes-10.png 
  
  margins:
  
  - left: 10
  - right: 4
  - top,bottom: 3
  
  ---
  
  > The new margins shown in the screenshot are just too huge for my taste.
  
  Well, it's hard to target each personal taste. For me, bigger left margin 
feels more natural. Typical the "for my taste" problem.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Marijo Mustac]

mmustac added a comment.


  I like the margins how they are at the moment and personally would increase 
it maximum by 5 px or so.
  The new margins shown in the screenshot are just too huge for my taste.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Vlad Zagorodniy]

zzag added a comment.


  > What other DE or OS that you used has such a wide menu that it feels 
natural to you?
  
  WIndows and macOS.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Vlad Zagorodniy]

zzag added a comment.


  @ngraham, @abetts
  
  F5706528: without-checkboxes.png 
  
  F5706532: with-checkboxes.png 
  
  Margins:
  
  - left: 20
  - right: 6
  - top: 3
  - bottom: 3
  
  What do you think of this?
  
  > This could work, just the left padding is too much in this patch. Maybe 
reduce by a 3rd? or Half?
  
  In my opinion, it should be even bigger. (regarding 16px(?) left padding)
  
  ---
  
  @anemeth see screenshots above

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Vlad Zagorodniy]

zzag updated this revision to Diff 26980.
zzag added a comment.


  allow changing individual margins(e.g. left/right/top/bottom margin)

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10438?vs=26906&id=26980

BRANCH
  menuitem-margins

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

AFFECTED FILES
  kstyle/breeze.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h

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

D10438: increase left/right margin of menuitems

[Andres Betts]

abetts added a comment.


  This could work, just the left padding is too much in this patch. Maybe 
reduce by a 3rd? or Half?

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Nathaniel Graham]

ngraham added a comment.


  I could //maybe// see this working for the left edge, but having such a lot 
of padding between the right edge and the arrows seems a little weird to me.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Alex Nemeth]

anemeth added a comment.


  How does it look when a menu has a checkbox on it, for example the 'View' 
menu on Konsole
  F5705719: Screenshot_20180212_011947.png 

  What other DE or OS that you used has such a wide menu that it feels natural 
to you?

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Vlad Zagorodniy]

zzag edited the summary of this revision.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Vlad Zagorodniy]

zzag edited the summary of this revision.

REPOSITORY
  R31 Breeze

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

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

D10438: increase left/right margin of menuitems

[Vlad Zagorodniy]

zzag created this revision.
zzag added reviewers: Breeze, VDG, ngraham, hpereiradacosta.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
zzag requested review of this revision.

REVISION SUMMARY
  At a given moment, there is a small left/right margin in menu items.
  This feels not really comfortable or "natural"(I don't know how to 
  express this feeling).
  
  Before(add image)
  
  After(add image)

TEST PLAN
  - open context menu

REPOSITORY
  R31 Breeze

BRANCH
  menuitem-margins

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

AFFECTED FILES
  kstyle/breeze.h
  kstyle/breezestyle.cpp

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