D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Nathaniel Graham]

ngraham added a comment.


  Fixed in D25885 

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: mart, KonqiDragon, ngraham, davidedmundson, filipf, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Marco Martin]

mart added a comment.


  In D22359#536077 , @ndavis wrote:
  
  > In D22359#536055 , @ngraham 
wrote:
  >
  > > Is there any way we can preserve the original goal of using a monochrome 
icon here for small sizes?
  >
  >
  > Maybe we could add an option to force a specific icon size. It would be 
literally the only option in the widget, but it would do the job and we could 
set a monochrome icon size by default. These applets should probably still get 
refactored some day though.
  >
  > It seems like setting specific sizes for specific widgets should be a 
feature of the panel editing mode though. That way people could set any size 
for any widget. I don't think our current way of allowing users to set the 
global panel icon size is very discoverable and it's quite limiting.
  
  
  I hope it doesn't come to that, add an option is a very KDE3 solution.
  that icon kinda needs to be smaller when it's placed in its intended default 
position.
  perhaps not when is somewhere else, i don't know
  
  but this revert makes the default just appear... broken

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: mart, KonqiDragon, ngraham, davidedmundson, filipf, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Noah Davis]

ndavis added a comment.


  In D22359#536055 , @ngraham wrote:
  
  > Is there any way we can preserve the original goal of using a monochrome 
icon here for small sizes?
  
  
  Maybe we could add an option to force a specific icon size. It would be 
literally the only option in the widget, but it would do the job and we could 
set a monochrome icon size by default. These applets should probably still get 
refactored some day though.

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: KonqiDragon, ngraham, davidedmundson, filipf, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[KonqiDragon]

KonqiDragon added a comment.


  In D22359#536066 , @ngraham wrote:
  
  > In D22359#536064 , @KonqiDragon 
wrote:
  >
  > > In D22359#536063 , @ngraham 
wrote:
  > >
  > > > In D22359#536057 , @ndavis 
wrote:
  > > >
  > > > > In D22359#536055 , 
@ngraham wrote:
  > > > >
  > > > > > Is there any way we can preserve the original goal of using a 
monochrome icon here for small sizes?
  > > > >
  > > > >
  > > > > Yes, it is already done. Lower the panel icon size to 16 or 22 px.
  > > >
  > > >
  > > > That doesn't help the huge fraction of people who never touch the 
default settings though. What's especially silly IMO is that the default panel 
height is 36px, at which size everything with a colorful icon version gets the 
colorful version... but if you reduce the panel height just one tick to 34px, 
the icons get smaller and everything becomes monochrome and it all looks great.
  > > >
  > > > I feel like we should either slightly reduce the default height of the 
panel to 34px, or change its icon sizing logic so that the threshold for 
everything turning colorful happens at some size that's greater than 36px.
  > >
  > >
  > > Why the Notifications, Volume, Network, etc. icons are monochrome, but 
"Show desktop" and "Minimize all" icons is hard to make a monochrome?
  >
  >
  > Those are in the System Tray, which always shows monochrome icons. Show 
Desktop is a standalone widget, which has logic to switch its icon between 
monochrome and colorful icons depending on the size of the container that the 
icon is inside.
  
  
  Maybe need to change the behavior of changing of the "Show desktop" and 
"Minimize all" icons when the panel is really big?

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: KonqiDragon, ngraham, davidedmundson, filipf, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Noah Davis]

ndavis added a comment.


  In D22359#536066 , @ngraham wrote:
  
  > Those are in the System Tray, which always shows monochrome icons. Show 
Desktop is a standalone widget, which has logic to switch its icon between 
monochrome and colorful icons depending on the size of the container that the 
icon is inside.
  
  
  It doesn't contain that logic in the code, it's just only capable of getting 
icons from the icon theme, for whatever reason. The icon theme happens to have 
monochrome icons at 16 & 22px and color at 32 & 64 px.

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: KonqiDragon, ngraham, davidedmundson, filipf, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Noah Davis]

ndavis added a comment.


  In D22359#536064 , @KonqiDragon 
wrote:
  
  > In D22359#536063 , @ngraham 
wrote:
  >
  > > In D22359#536057 , @ndavis 
wrote:
  > >
  > > > In D22359#536055 , @ngraham 
wrote:
  > > >
  > > > > Is there any way we can preserve the original goal of using a 
monochrome icon here for small sizes?
  > > >
  > > >
  > > > Yes, it is already done. Lower the panel icon size to 16 or 22 px.
  > >
  > >
  > > That doesn't help the huge fraction of people who never touch the default 
settings though. What's especially silly IMO is that the default panel height 
is 36px, at which size everything with a colorful icon version gets the 
colorful version... but if you reduce the panel height just one tick to 34px, 
the icons get smaller and everything becomes monochrome and it all looks great.
  > >
  > > I feel like we should either slightly reduce the default height of the 
panel to 34px, or change its icon sizing logic so that the threshold for 
everything turning colorful happens at some size that's greater than 36px.
  >
  >
  > Why the Notifications, Volume, Network, etc. icons are monochrome, but 
"Show desktop" and "Minimize all" icons is hard to make a monochrome?
  >  Colored icon of "Show desktop" and "Minimize all" is bigger that other 
monochrome icons.
  
  
  Because they're written differently ¯\_(ツ)_/¯.

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: KonqiDragon, ngraham, davidedmundson, filipf, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Nathaniel Graham]

ngraham added a comment.


  In D22359#536064 , @KonqiDragon 
wrote:
  
  > In D22359#536063 , @ngraham 
wrote:
  >
  > > In D22359#536057 , @ndavis 
wrote:
  > >
  > > > In D22359#536055 , @ngraham 
wrote:
  > > >
  > > > > Is there any way we can preserve the original goal of using a 
monochrome icon here for small sizes?
  > > >
  > > >
  > > > Yes, it is already done. Lower the panel icon size to 16 or 22 px.
  > >
  > >
  > > That doesn't help the huge fraction of people who never touch the default 
settings though. What's especially silly IMO is that the default panel height 
is 36px, at which size everything with a colorful icon version gets the 
colorful version... but if you reduce the panel height just one tick to 34px, 
the icons get smaller and everything becomes monochrome and it all looks great.
  > >
  > > I feel like we should either slightly reduce the default height of the 
panel to 34px, or change its icon sizing logic so that the threshold for 
everything turning colorful happens at some size that's greater than 36px.
  >
  >
  > Why the Notifications, Volume, Network, etc. icons are monochrome, but 
"Show desktop" and "Minimize all" icons is hard to make a monochrome?
  
  
  Those are in the System Tray, which always shows monochrome icons. Show 
Desktop is a standalone widget, which has logic to switch its icon between 
monochrome and colorful icons depending on the size of the container that the 
icon is inside.

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: KonqiDragon, ngraham, davidedmundson, filipf, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[KonqiDragon]

KonqiDragon added a comment.


  In D22359#536063 , @ngraham wrote:
  
  > In D22359#536057 , @ndavis wrote:
  >
  > > In D22359#536055 , @ngraham 
wrote:
  > >
  > > > Is there any way we can preserve the original goal of using a 
monochrome icon here for small sizes?
  > >
  > >
  > > Yes, it is already done. Lower the panel icon size to 16 or 22 px.
  >
  >
  > That doesn't help the huge fraction of people who never touch the default 
settings though. What's especially silly IMO is that the default panel height 
is 36px, at which size everything with a colorful icon version gets the 
colorful version... but if you reduce the panel height just one tick to 34px, 
the icons get smaller and everything becomes monochrome and it all looks great.
  >
  > I feel like we should either slightly reduce the default height of the 
panel to 34px, or change its icon sizing logic so that the threshold for 
everything turning colorful happens at some size that's greater than 36px.
  
  
  Why the Notifications, Volume, Network, etc. icons are monochrome, but "Show 
desktop" and "Minimize all" icons is hard to make a monochrome?

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: KonqiDragon, ngraham, davidedmundson, filipf, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Nathaniel Graham]

ngraham added a comment.


  In D22359#536057 , @ndavis wrote:
  
  > In D22359#536055 , @ngraham 
wrote:
  >
  > > Is there any way we can preserve the original goal of using a monochrome 
icon here for small sizes?
  >
  >
  > Yes, it is already done. Lower the panel icon size to 16 or 22 px.
  
  
  That doesn't help the huge fraction of people who never touch the default 
settings though. What's especially silly IMO is that the default panel height 
is 36px, at which size everything with a colorful icon version gets the 
colorful version... but if you reduce the panel height just one tick to 34px, 
the icons get smaller and everything becomes monochrome and it all looks great.
  
  I feel like we should either slightly reduce the default height of the panel 
to 34px, or change its icon sizing logic so that the threshold for everything 
turning colorful happens at some size that's greater than 36px.

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: KonqiDragon, ngraham, davidedmundson, filipf, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Noah Davis]

ndavis added a comment.


  In D22359#536058 , @KonqiDragon 
wrote:
  
  > I really don't like that in the Plasma 5.17 Beta you returned a colored 
icon, when in Plasma 5.16 you added a monochrome icon i was glad. I don't like 
this regression. We need to talk with #consistency 
 and #vdg 
 about how to return and make a 
monochrome icon better.
  
  
  I cannot even make the applets use monochrome icons from the desktop theme. 
Most likely, these 2 applets need to be refactored and I don't know how to do 
that.

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: KonqiDragon, ngraham, davidedmundson, filipf, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[KonqiDragon]

KonqiDragon added a comment.


  I really don't like that in the Plasma 5.17 Beta you returned a colored icon, 
when in Plasma 5.16 you added a monochrome icon i was glad. I don't like this 
regression. We need to talk with #consistency 
 and #vdg 
 about how to return and make a 
monochrome icon better.

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: KonqiDragon, ngraham, davidedmundson, filipf, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Noah Davis]

ndavis added a comment.


  In D22359#536055 , @ngraham wrote:
  
  > Is there any way we can preserve the original goal of using a monochrome 
icon here for small sizes?
  
  
  Yes, it is already done. Lower the panel icon size to at least 22px.

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: ngraham, davidedmundson, filipf, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Nathaniel Graham]

ngraham added a comment.


  Is there any way we can preserve the original goal of using a monochrome icon 
here for small sizes?

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: ngraham, davidedmundson, filipf, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Noah Davis]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:9705414e3ec3: Revert "[showdesktop][minimizeall] 
Reduce the maximum panel icon size" (authored by ndavis).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22359?vs=61468&id=64690

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

AFFECTED FILES
  applets/minimizeall/package/contents/ui/main.qml
  applets/showdesktop/package/contents/ui/main.qml

To: ndavis, #vdg, #plasma, davidedmundson
Cc: davidedmundson, filipf, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Noah Davis]

ndavis added a comment.


  Seems like people still complain about the patch, so I'll go ahead and land 
this.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  revert-showdesktop-minimizall (branched from master)

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: davidedmundson, filipf, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Noah Davis]

ndavis added a comment.


  I was hesitant about making this change before I committed it, so it's not a 
knee jerk reaction, but I also don't think it's important for me to land this 
quickly. I might want to see if I can come up with the real solution.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  revert-showdesktop-minimizall (branched from master)

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: davidedmundson, filipf, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[David Edmundson]

davidedmundson accepted this revision.
davidedmundson added a comment.
This revision is now accepted and ready to land.


  > Some users complained about the buttons being smaller or the size not being 
configurable when it previously was.
  
  Whatever you change some users will always complain. 
  It's important not to knee-jerk react based on some comments if they go 
against the general trend.
  
  Code-wise it's no worse than before your patches, so I'm happy to accept, but 
if you want to keep it as-is, I would support that too.
  
  Your call.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  revert-showdesktop-minimizall (branched from master)

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

To: ndavis, #vdg, #plasma, davidedmundson
Cc: davidedmundson, filipf, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Noah Davis]

ndavis added a comment.


  In D22359#504108 , @davidedmundson 
wrote:
  
  > Can you give a bit more rationale?
  
  
  Some users complained about the buttons being smaller or the size not being 
configurable when it previously was.

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma
Cc: davidedmundson, filipf, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[David Edmundson]

davidedmundson added a comment.


  Can you give a bit more rationale?

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma
Cc: davidedmundson, filipf, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Filip Fila]

filipf added a comment.


  I don't feel too wise regarding this, pinging #plasma 
 in case someone has advice.

REPOSITORY
  R119 Plasma Desktop

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

To: ndavis, #vdg, #plasma
Cc: filipf, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22359: Revert "[showdesktop][minimizeall] Reduce the maximum panel icon size"

[Noah Davis]

ndavis created this revision.
ndavis added reviewers: VDG, Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
ndavis requested review of this revision.

REVISION SUMMARY
  This reverts the following commits:
  fff556af1939d17453540f4388b27650359258dd 

  5de082c39b91f804fa6d50bfa9df9f7463d286dd 

  33be0f5b8b877a8a3bf99752ababf38b54d9145d 

  
  I think I did this change the wrong way. What we really need is a way to use 
different sizes for different panel widgets by default without making it 
impossible for users to configure the size.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  revert-showdesktop-minimizall (branched from master)

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

AFFECTED FILES
  applets/minimizeall/package/contents/ui/main.qml
  applets/showdesktop/package/contents/ui/main.qml

To: ndavis, #vdg, #plasma
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart