D29603: Adds custom spacing between task manager icons

[Nathaniel Graham]

ngraham commandeered this revision.
ngraham edited reviewers, added: martellini; removed: ngraham.
ngraham added a comment.


  Cool, thanks!

REPOSITORY
  R119 Plasma Desktop

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

To: ngraham, #vdg, martellini
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Jacopo Martellini]

martellini added a comment.


  Done
  Adjustable spacing between icons 


REPOSITORY
  R119 Plasma Desktop

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

To: martellini, #vdg, ngraham
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Nathaniel Graham]

ngraham added a comment.


  Thanks! Do you think you could maybe re-open this at 
https://invent.kde.org/plasma/plasma-desktop/-/merge_requests/? We're 
transitioning patch review over to GitLab.

REPOSITORY
  R119 Plasma Desktop

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

To: martellini, #vdg, ngraham
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Jacopo Martellini]

martellini updated this revision to Diff 83182.
martellini added a comment.


  Changed the input field from SpinBox to ComboBox and the options are:
  
  - Narrow: 0 px
  - Medium: 8 px
  - Wide:   16 px
  - Extra wide: 24 px

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29603?vs=82986&id=83182

BRANCH
  adjustable-spacing-between-icons (branched from master)

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

AFFECTED FILES
  applets/taskmanager/package/contents/config/main.xml
  applets/taskmanager/package/contents/ui/ConfigAppearance.qml
  applets/taskmanager/package/contents/ui/Task.qml
  applets/taskmanager/package/contents/ui/code/layout.js

To: martellini, #vdg, ngraham
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Jacopo Martellini]

martellini added a comment.


  In D29603#673942 , @ngraham wrote:
  
  > Thanks! And sorry for the long delay before reviewing. However this doesn't 
seem to be working for me; no matter what value I enter, my vertical 
icons-only-task manager's icon spacing does not change.
  >
  > Also, instead of allowing the user to specify a pixel value, what do you 
think about using more general terminology and a combobox? For example:
  >
  > - "Narrow" => 2px spacing
  > - "Medium" => 4px spacing
  > - "Wide" => 8 px spacing
  > - "Extra wide" => 12 px spacing
  >
  >   (numbers are totally random, just for example purposes)
  >
  >   What do you think?
  
  
  Hi, don't worry, I imagine this has been a busy period with 5.19 coming and 
the migration to Gitlab.
  Ok, I'm writing your idea, but with values from 0 to 24.
  Btw I think I've bumped into this: ComboBox.valueRole is not available 
 
  It should have been solved, but on neon with kde 5.18.5, kf 5.69, and qt 
5.14.2 it still seems broken. Maybe I should see if upgrading solves the issue, 
but I'm skipping 5.70 because of that time units issue.
  I'll keep trying and let you know

REPOSITORY
  R119 Plasma Desktop

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

To: martellini, #vdg, ngraham
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Nathaniel Graham]

ngraham added a comment.


  Thanks! And sorry for the long delay before reviewing. However this doesn't 
seem to be working for me; no matter what value I enter, my vertical 
icons-only-task manager's icon spacing does not change.
  
  Also, instead of allowing the user to specify a pixel value, what do you 
think about using more general terminology and a combobox? For example:
  
  - "Narrow" => 2px spacing
  - "Medium" => 4px spacing
  - "Wide" => 8 px spacing
  - "Extra wide" => 12 px spacing
  
  (numbers are totally random, just for example purposes)
  
  What do you think?

REPOSITORY
  R119 Plasma Desktop

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

To: martellini, #vdg, ngraham
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Jacopo Martellini]

martellini added a comment.


  It should be a little better now, but I left out 2 of the things we said:
  
  - I didn't remove the icon size setting from the configuration of the 
vertical task manager because maybe someone likes small icons when the panel is 
by the side
  - I left the spacing setting in pixels because I don't know how wide should 
the wide setting be and I imagined people asking for wider or narrower spacing
  
  However, what do you think?

REPOSITORY
  R119 Plasma Desktop

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

To: martellini, #vdg, ngraham
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Jacopo Martellini]

martellini updated this revision to Diff 82986.
martellini added a comment.


  - Separates the Spacing setting from the others

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29603?vs=82983&id=82986

BRANCH
  adjustable-spacing-between-icons (branched from master)

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

AFFECTED FILES
  applets/taskmanager/package/contents/config/main.xml
  applets/taskmanager/package/contents/ui/ConfigAppearance.qml
  applets/taskmanager/package/contents/ui/Task.qml
  applets/taskmanager/package/contents/ui/code/layout.js

To: martellini, #vdg, ngraham
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Jacopo Martellini]

martellini updated this revision to Diff 82983.
martellini added a comment.


  - Makes icon scale linearly to the size of the taksbar
  - Improved the spacing configuration of the taskbar icons

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29603?vs=82452&id=82983

BRANCH
  adjustable-spacing-between-icons (branched from master)

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

AFFECTED FILES
  applets/taskmanager/package/contents/config/main.xml
  applets/taskmanager/package/contents/ui/ConfigAppearance.qml
  applets/taskmanager/package/contents/ui/Task.qml
  applets/taskmanager/package/contents/ui/code/layout.js

To: martellini, #vdg, ngraham
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Jacopo Martellini]

martellini added a comment.


  In D29603#669457 , @ngraham wrote:
  
  > In D29603#669015 , @martellini 
wrote:
  >
  > > Sounds cool. I'll be working on it.
  > >  I must that the iconSize setting is only visibile if the panel is 
vertical, why do you think it is so?
  >
  >
  > Not sure I fully understahd, but it should ideally be visible for both 
vertical and horizontal panels.
  
  
  Sorry, my bad, there was a verb missing: *I must //say// etc
  Anyway that setting is specific for the vertical panel: you can try to move 
your panel to the top/bottom and you wont see it, nor it will affect the panel.
  I'll break something and let you know

REPOSITORY
  R119 Plasma Desktop

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

To: martellini, #vdg, ngraham
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Nathaniel Graham]

ngraham added a comment.


  In D29603#669015 , @martellini 
wrote:
  
  > Sounds cool. I'll be working on it.
  >  I must that the iconSize setting is only visibile if the panel is 
vertical, why do you think it is so?
  
  
  Not sure I fully understahd, but it should ideally be visible for both 
vertical and horizontal panels.

REPOSITORY
  R119 Plasma Desktop

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

To: martellini, #vdg, ngraham
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Jacopo Martellini]

martellini added a comment.


  Sounds cool. I'll be working on it.
  I must that the iconSize setting is only visibile if the panel is vertical, 
why do you think it is so?

REPOSITORY
  R119 Plasma Desktop

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

To: martellini, #vdg, ngraham
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Nathaniel Graham]

ngraham added a comment.


  In D29603#668422 , @martellini 
wrote:
  
  > > Also, it doesn't make sense IMO to expose separate horizontal and 
vertical controls; there should be just one control visible, and it should 
affect the task manager no matter its orientation.
  >
  > If that's the better way I totally agree, but I don't know recall that 
setting. Is there an open issue that I can look into?
  
  
  This thing: F8310217: this thing 
  
  Right now IMO its function is pretty unclear and useless. Adjusting it 
usually doesn't change anything at all, making it look broken. I think we 
should fix this first.
  
  Conceptually, I think by default, the icon size should smoothly scale with 
the panel thickness. Right now it roughly does this, but with hard sizes rather 
than smoothly scaling the icons, as the macOS Dock does. I think that would be 
a better default.
  
  Then, on top of that, we could additionally have the following settings for 
fine-tuning:
  
  - Spacing (narrow or wide)
  - Maximum icon size (or maybe we don't even need this at all?)
  
  How does that sound?

REPOSITORY
  R119 Plasma Desktop

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

To: martellini, #vdg, ngraham
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Jacopo Martellini]

martellini added a comment.


  In D29603#668283 , @niccolove 
wrote:
  
  > Are we really sure this is worth exposing to the user?
  
  
  Not sure at all. I just can't stand the default spacing, because it feels 
like the icons are crammed. At least if compared with Win10 or Gnome with dash 
to panel.
  
  In D29603#668297 , @ngraham wrote:
  
  > I think it would make more sense to fix the default sizing behavior rather 
than adding these. We already have an "icon size" setting which roughly adjusts 
this already, just not very well. That should be fixed rather than adding 
additional user-facing settings, or at least before doing so.
  >
  > Also, it doesn't make sense IMO to expose separate horizontal and vertical 
controls; there should be just one control visible, and it should affect the 
task manager no matter its orientation.
  
  
  If that's the better way I totally agree, but I don't know recall that 
setting. Is there an open issue that I can look into?

REPOSITORY
  R119 Plasma Desktop

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

To: martellini, #vdg, ngraham
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  I think it would make more sense to fix the default sizing behavior rather 
than adding these. We already have an "icon size" setting which roughly adjusts 
this already, just not very well. That should be fixed rather than adding 
additional user-facing settings, or at least before doing so.
  
  Also, it doesn't make sense IMO to expose separate horizontal and vertical 
controls; there should be just one control visible, and it should affect the 
task manager no matter its orientation.

REPOSITORY
  R119 Plasma Desktop

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

To: martellini, #vdg, ngraham
Cc: ngraham, niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Niccolò Venerandi]

niccolove added a comment.


  Are we really sure this is worth exposing to the user?

REPOSITORY
  R119 Plasma Desktop

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

To: martellini, #vdg
Cc: niccolove, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29603: Adds custom spacing between task manager icons

[Jacopo Martellini]

martellini created this revision.
martellini added a reviewer: VDG.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
martellini requested review of this revision.

REVISION SUMMARY
  Adds the option to set a custom spacing between the icons of the icons-only 
task manager.
  Both horizontal and vertical spacing can be set.
  Probably there should be an upper limit, but I don't know what it could be.
  
  BUG: 363878

REPOSITORY
  R119 Plasma Desktop

BRANCH
  adjustable-spacing-between-icons (branched from master)

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

AFFECTED FILES
  applets/taskmanager/package/contents/config/main.xml
  applets/taskmanager/package/contents/ui/ConfigAppearance.qml
  applets/taskmanager/package/contents/ui/code/layout.js

To: martellini, #vdg
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart