Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/
---

(Updated July 14, 2016, 11:04 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and KDE Usability.


Changes
---

Submitted with commit 7f060403a2d44fc76b2b14fc9ebf43ee272d1c54 by Marco Martin 
to branch master.


Repository: plasma-workspace


Description
---

Configuration option for system tray's icon sizes in order to be handled by 
look & feel themes/packages.


Diffs
-

  applets/systemtray/package/contents/config/main.xml 65a7029 
  applets/systemtray/package/contents/ui/main.qml a66ea69 

Diff: https://git.reviewboard.kde.org/r/128400/diff/


Testing
---

Tested in KDE Neon Developer Stable (as of July 7, 2016)


Thanks,

John Salatas

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Marco Martin]

> On July 14, 2016, 10:33 a.m., Viorel-Cătălin Răpițeanu wrote:
> > Is there any status update on this commit? Should it be integrated?
> 
> John Salatas wrote:
> I'm done with it. Hopefully someone will commit to git as I don't have 
> commit rights in order to do it myself.

i an commit it, but if oyu are interested, you can request for commit rights :)


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97384
---


On July 12, 2016, 8:29 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 12, 2016, 8:29 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Configuration option for system tray's icon sizes in order to be handled by 
> look & feel themes/packages.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

> On July 14, 2016, 10:33 a.m., Viorel-Cătălin Răpițeanu wrote:
> > Is there any status update on this commit? Should it be integrated?

I'm done with it. Hopefully someone will commit to git as I don't have commit 
rights in order to do it myself.


- John


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97384
---


On July 12, 2016, 8:29 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 12, 2016, 8:29 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Configuration option for system tray's icon sizes in order to be handled by 
> look & feel themes/packages.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Viorel-Cătălin Răpițeanu]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97384
---



Is there any status update on this commit? Should it be integrated?

- Viorel-Cătălin Răpițeanu


On Iulie 12, 2016, 11:29 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated Iulie 12, 2016, 11:29 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Configuration option for system tray's icon sizes in order to be handled by 
> look & feel themes/packages.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

> On July 12, 2016, 11:40 a.m., Kai Uwe Broulik wrote:
> >

done


- John


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97313
---


On July 12, 2016, 8:29 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 12, 2016, 8:29 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Configuration option for system tray's icon sizes in order to be handled by 
> look & feel themes/packages.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Kai Uwe Broulik]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97313
---




applets/systemtray/package/contents/ui/main.qml (line 33)


Use property var instead of variant


- Kai Uwe Broulik


On Juli 12, 2016, 8:29 vorm., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated Juli 12, 2016, 8:29 vorm.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Configuration option for system tray's icon sizes in order to be handled by 
> look & feel themes/packages.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

> On July 12, 2016, 7:46 a.m., Marco Martin wrote:
> >
> 
> John Salatas wrote:
> done

I just realized that I cannot commit to git and (obviously) marking it as 
submitted it doesn't submit it :)

Can someone submit it for me? 

Thanks!


- John


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97305
---


On July 12, 2016, 8:29 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 12, 2016, 8:29 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Configuration option for system tray's icon sizes in order to be handled by 
> look & feel themes/packages.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/
---

(Updated July 12, 2016, 8:29 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and KDE Usability.


Repository: plasma-workspace


Description
---

Configuration option for system tray's icon sizes in order to be handled by 
look & feel themes/packages.


Diffs
-

  applets/systemtray/package/contents/config/main.xml 65a7029 
  applets/systemtray/package/contents/ui/main.qml a66ea69 

Diff: https://git.reviewboard.kde.org/r/128400/diff/


Testing
---

Tested in KDE Neon Developer Stable (as of July 7, 2016)


Thanks,

John Salatas

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97308
---


Ship it!




Ship It!

- John Salatas


On July 8, 2016, 8:56 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:56 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Configuration option for system tray's icon sizes in order to be handled by 
> look & feel themes/packages.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

> On July 12, 2016, 7:46 a.m., Marco Martin wrote:
> >

done


- John


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97305
---


On July 8, 2016, 8:56 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:56 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Configuration option for system tray's icon sizes in order to be handled by 
> look & feel themes/packages.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97305
---


Fix it, then Ship it!





applets/systemtray/package/contents/ui/main.qml (line 35)


since iconSizeFromTheme() is used in only one place, maybe the function can 
just be declared inline under the MouseArea instead of having a dedicated file?


- Marco Martin


On July 8, 2016, 8:56 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:56 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Configuration option for system tray's icon sizes in order to be handled by 
> look & feel themes/packages.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

> On July 8, 2016, 11:21 a.m., Thomas Pfeiffer wrote:
> > Visual aspects should stay under the theme's control. Therefore, from the 
> > usability perspective, there should be no user option, instead the look & 
> > feel theme should be able to control the icon size.
> 
> John Salatas wrote:
> I don't think that currently the theme controls it. It scales to follow 
> the panel's size up to a certain size which is hardcoded: 
> units.iconSizes.medium
> 
> Why units.iconSizes.medium and not units.iconSizes.smallMedium? Is this a 
> design decision or just a random pickup? :\
> 
> What I'm proposing here is let the user choose instead of having an 
> arbitrary maximum size hardcoded. :\
> 
> Marco Martin wrote:
> i do think smallmedium would be a better default.
> in order to do as Thomas suggests, it can have the configuration value, 
> but not exposed from the config dialog (and yeah, defaulting to smallmedium).
> then, look and feel packages can contain default setups for applets, it 
> would be that one that make it theme-controlled.
> 
> Also, i would save in the config smallmedium/medium etc, instead of a 
> pixel size value, in order to still survive DPI changes, screen migration etc.

OK. I just updated the diff file. The system tray now has the iconSize option 
which is not exposed from the config dialog. 

Look and feel packages can set this value according to the documentation at 
https://userbase.kde.org/KDE_System_Administration/PlasmaTwoDesktopScripting#Default_applets_config_from_Look_and_Feel_packages

That is by creating a file name org.kde.plasma.private.systemtray.js in the 
plasmoidsetupscripts subfolder of the Look and Feel package with the following 
contents

applet.currentConfigGroup = new Array("General");
applet.writeConfig("iconSize", 1);
applet.reloadConfig();

Of course a user can still modify this value by manually editing his 
~/.config/plasma-org.kde.plasma.desktop-appletsrc


- John


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97212
---


On July 8, 2016, 8:56 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:56 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Configuration option for system tray's icon sizes in order to be handled by 
> look & feel themes/packages.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Marco Martin]

> On July 8, 2016, 11:21 a.m., Thomas Pfeiffer wrote:
> > Visual aspects should stay under the theme's control. Therefore, from the 
> > usability perspective, there should be no user option, instead the look & 
> > feel theme should be able to control the icon size.
> 
> John Salatas wrote:
> I don't think that currently the theme controls it. It scales to follow 
> the panel's size up to a certain size which is hardcoded: 
> units.iconSizes.medium
> 
> Why units.iconSizes.medium and not units.iconSizes.smallMedium? Is this a 
> design decision or just a random pickup? :\
> 
> What I'm proposing here is let the user choose instead of having an 
> arbitrary maximum size hardcoded. :\

i do think smallmedium would be a better default.
in order to do as Thomas suggests, it can have the configuration value, but not 
exposed from the config dialog (and yeah, defaulting to smallmedium).
then, look and feel packages can contain default setups for applets, it would 
be that one that make it theme-controlled.

Also, i would save in the config smallmedium/medium etc, instead of a pixel 
size value, in order to still survive DPI changes, screen migration etc.


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97212
---


On July 8, 2016, 8:56 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:56 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

> On July 8, 2016, 11:21 a.m., Thomas Pfeiffer wrote:
> > Visual aspects should stay under the theme's control. Therefore, from the 
> > usability perspective, there should be no user option, instead the look & 
> > feel theme should be able to control the icon size.

I don't think that currently the theme controls it. It scales to follow the 
panel's size up to a certain size which is hardcoded: units.iconSizes.medium

Why units.iconSizes.medium and not units.iconSizes.smallMedium? Is this a 
design decision or just a random pickup? :\

What I'm proposing here is let the user choose instead of having an arbitrary 
maximum size hardcoded. :\


- John


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97212
---


On July 8, 2016, 8:56 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:56 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Thomas Pfeiffer]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97212
---



Visual aspects should stay under the theme's control. Therefore, from the 
usability perspective, there should be no user option, instead the look & feel 
theme should be able to control the icon size.

- Thomas Pfeiffer


On July 8, 2016, 8:56 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:56 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Gregor Mi]

> On July 8, 2016, 7:21 a.m., Kai Uwe Broulik wrote:
> > Thanks for your patch!
> > 
> > However, I don't think there should be such an option. There's no other 
> > applet which has this and applets in the panel usually follow the panel 
> > size indefinitely. Perhaps add the "usability" group to have them decide.
> > 
> > Also, I don't think this single option deserves a separage page, it would 
> > fit just fine at the top of the "General" page imho.
> 
> John Salatas wrote:
> Thanks for your feedback! 
> 
> I updated the diff and I believe I have addressed all of the issues you 
> pointed.
> 
> I also added the usability group. 
> 
> IMHO I don't think having this option is bad as I guess that the 
> "correct" icon size is depended to the monitor so maybe it is better to be 
> controlled by the user instead of having a fixed value :\
> 
> Marco Martin wrote:
> well, depending from the monitor means it should depend from the monitor 
> dpi, rather than the user (and yes, medium size was definitely too big, but i 
> don't think it grants an option to explicitly control this)
> 
> Kai Uwe Broulik wrote:
> iconSizes are scaled with DPI, no?
> 
> Marco Martin wrote:
> yes they are
> 
> John Salatas wrote:
> My "problem" that medium size seems to be too large for me and according 
> to the bug #364431 this doesn't seem to be the case for all users. So I 
> though that it would be better if the size was controled by the user :\
> 
> FYI: I have two monitors: one 15 inch and one 24 Inch both with 1920x1080 
> resolution and the medium size icons seems too large in both of them :\
> 
> Kai Uwe Broulik wrote:
> It can be controlled, by changing the panel size. If the size changed 
> between Plasma 5.6 and 5.7 that's a bug. If it was large before already then 
> this is expected and as it should be.
> 
> John Salatas wrote:
> Indeed, there was a change from 5.6 to 5.7 (it was large before). 
> 
> In any case, can't we just consider what I'm proposing here (the ability 
> to change the icon size in the systray) as a new feature? :)

On my system (openSUSE 42.1 laptop with external monitor), I also noticed the 
change from 5.6 to 5.7. The tray icons are now smaller. I found the larger ones 
more comfortable to deal with.


- Gregor


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97186
---


On July 8, 2016, 8:56 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:56 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

> On July 8, 2016, 9:45 a.m., Heiko Tietze wrote:
> > Please add a screenshot. But actually I wonder why size matters. Aren't 
> > icons adjusted according the panel height anymore? That would by a minus 
> > one.
> 
> Kai Uwe Broulik wrote:
> They are. For system tray only up to a certain point, though, at which it 
> will become two or more rows, like with task manager, so the tray doesn't 
> fill half the panel width. This patch adds a setting to define the maximum 
> size for the icons.

https://bugsfiles.kde.org/attachment.cgi?id=99911

The upper one is with medium size which I find it too big.

I would preffer the smallMedium size (lower one).


- John


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97202
---


On July 8, 2016, 8:56 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:56 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Kai Uwe Broulik]

> On Juli 8, 2016, 9:45 vorm., Heiko Tietze wrote:
> > Please add a screenshot. But actually I wonder why size matters. Aren't 
> > icons adjusted according the panel height anymore? That would by a minus 
> > one.

They are. For system tray only up to a certain point, though, at which it will 
become two or more rows, like with task manager, so the tray doesn't fill half 
the panel width. This patch adds a setting to define the maximum size for the 
icons.


- Kai Uwe


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97202
---


On Juli 8, 2016, 8:56 vorm., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated Juli 8, 2016, 8:56 vorm.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Heiko Tietze]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97202
---



Please add a screenshot. But actually I wonder why size matters. Aren't icons 
adjusted according the panel height anymore? That would by a minus one.

- Heiko Tietze


On July 8, 2016, 8:56 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:56 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

> On July 8, 2016, 7:21 a.m., Kai Uwe Broulik wrote:
> > Thanks for your patch!
> > 
> > However, I don't think there should be such an option. There's no other 
> > applet which has this and applets in the panel usually follow the panel 
> > size indefinitely. Perhaps add the "usability" group to have them decide.
> > 
> > Also, I don't think this single option deserves a separage page, it would 
> > fit just fine at the top of the "General" page imho.
> 
> John Salatas wrote:
> Thanks for your feedback! 
> 
> I updated the diff and I believe I have addressed all of the issues you 
> pointed.
> 
> I also added the usability group. 
> 
> IMHO I don't think having this option is bad as I guess that the 
> "correct" icon size is depended to the monitor so maybe it is better to be 
> controlled by the user instead of having a fixed value :\
> 
> Marco Martin wrote:
> well, depending from the monitor means it should depend from the monitor 
> dpi, rather than the user (and yes, medium size was definitely too big, but i 
> don't think it grants an option to explicitly control this)
> 
> Kai Uwe Broulik wrote:
> iconSizes are scaled with DPI, no?
> 
> Marco Martin wrote:
> yes they are
> 
> John Salatas wrote:
> My "problem" that medium size seems to be too large for me and according 
> to the bug #364431 this doesn't seem to be the case for all users. So I 
> though that it would be better if the size was controled by the user :\
> 
> FYI: I have two monitors: one 15 inch and one 24 Inch both with 1920x1080 
> resolution and the medium size icons seems too large in both of them :\
> 
> Kai Uwe Broulik wrote:
> It can be controlled, by changing the panel size. If the size changed 
> between Plasma 5.6 and 5.7 that's a bug. If it was large before already then 
> this is expected and as it should be.

Indeed, there was a change from 5.6 to 5.7 (it was large before). 

In any case, can't we just consider what I'm proposing here (the ability to 
change the icon size in the systray) as a new feature? :)


- John


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97186
---


On July 8, 2016, 8:56 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:56 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Kai Uwe Broulik]

> On Juli 8, 2016, 7:21 vorm., Kai Uwe Broulik wrote:
> > Thanks for your patch!
> > 
> > However, I don't think there should be such an option. There's no other 
> > applet which has this and applets in the panel usually follow the panel 
> > size indefinitely. Perhaps add the "usability" group to have them decide.
> > 
> > Also, I don't think this single option deserves a separage page, it would 
> > fit just fine at the top of the "General" page imho.
> 
> John Salatas wrote:
> Thanks for your feedback! 
> 
> I updated the diff and I believe I have addressed all of the issues you 
> pointed.
> 
> I also added the usability group. 
> 
> IMHO I don't think having this option is bad as I guess that the 
> "correct" icon size is depended to the monitor so maybe it is better to be 
> controlled by the user instead of having a fixed value :\
> 
> Marco Martin wrote:
> well, depending from the monitor means it should depend from the monitor 
> dpi, rather than the user (and yes, medium size was definitely too big, but i 
> don't think it grants an option to explicitly control this)
> 
> Kai Uwe Broulik wrote:
> iconSizes are scaled with DPI, no?
> 
> Marco Martin wrote:
> yes they are
> 
> John Salatas wrote:
> My "problem" that medium size seems to be too large for me and according 
> to the bug #364431 this doesn't seem to be the case for all users. So I 
> though that it would be better if the size was controled by the user :\
> 
> FYI: I have two monitors: one 15 inch and one 24 Inch both with 1920x1080 
> resolution and the medium size icons seems too large in both of them :\

It can be controlled, by changing the panel size. If the size changed between 
Plasma 5.6 and 5.7 that's a bug. If it was large before already then this is 
expected and as it should be.


- Kai Uwe


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97186
---


On Juli 8, 2016, 8:56 vorm., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated Juli 8, 2016, 8:56 vorm.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

> On July 8, 2016, 7:21 a.m., Kai Uwe Broulik wrote:
> > Thanks for your patch!
> > 
> > However, I don't think there should be such an option. There's no other 
> > applet which has this and applets in the panel usually follow the panel 
> > size indefinitely. Perhaps add the "usability" group to have them decide.
> > 
> > Also, I don't think this single option deserves a separage page, it would 
> > fit just fine at the top of the "General" page imho.
> 
> John Salatas wrote:
> Thanks for your feedback! 
> 
> I updated the diff and I believe I have addressed all of the issues you 
> pointed.
> 
> I also added the usability group. 
> 
> IMHO I don't think having this option is bad as I guess that the 
> "correct" icon size is depended to the monitor so maybe it is better to be 
> controlled by the user instead of having a fixed value :\
> 
> Marco Martin wrote:
> well, depending from the monitor means it should depend from the monitor 
> dpi, rather than the user (and yes, medium size was definitely too big, but i 
> don't think it grants an option to explicitly control this)
> 
> Kai Uwe Broulik wrote:
> iconSizes are scaled with DPI, no?
> 
> Marco Martin wrote:
> yes they are

My "problem" that medium size seems to be too large for me and according to the 
bug #364431 this doesn't seem to be the case for all users. So I though that it 
would be better if the size was controled by the user :\

FYI: I have two monitors: one 15 inch and one 24 Inch both with 1920x1080 
resolution and the medium size icons seems too large in both of them :\


- John


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97186
---


On July 8, 2016, 8:56 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:56 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Marco Martin]

> On July 8, 2016, 7:21 a.m., Kai Uwe Broulik wrote:
> > Thanks for your patch!
> > 
> > However, I don't think there should be such an option. There's no other 
> > applet which has this and applets in the panel usually follow the panel 
> > size indefinitely. Perhaps add the "usability" group to have them decide.
> > 
> > Also, I don't think this single option deserves a separage page, it would 
> > fit just fine at the top of the "General" page imho.
> 
> John Salatas wrote:
> Thanks for your feedback! 
> 
> I updated the diff and I believe I have addressed all of the issues you 
> pointed.
> 
> I also added the usability group. 
> 
> IMHO I don't think having this option is bad as I guess that the 
> "correct" icon size is depended to the monitor so maybe it is better to be 
> controlled by the user instead of having a fixed value :\
> 
> Marco Martin wrote:
> well, depending from the monitor means it should depend from the monitor 
> dpi, rather than the user (and yes, medium size was definitely too big, but i 
> don't think it grants an option to explicitly control this)
> 
> Kai Uwe Broulik wrote:
> iconSizes are scaled with DPI, no?

yes they are


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97186
---


On July 8, 2016, 8:56 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:56 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Kai Uwe Broulik]

> On Juli 8, 2016, 7:21 vorm., Kai Uwe Broulik wrote:
> > Thanks for your patch!
> > 
> > However, I don't think there should be such an option. There's no other 
> > applet which has this and applets in the panel usually follow the panel 
> > size indefinitely. Perhaps add the "usability" group to have them decide.
> > 
> > Also, I don't think this single option deserves a separage page, it would 
> > fit just fine at the top of the "General" page imho.
> 
> John Salatas wrote:
> Thanks for your feedback! 
> 
> I updated the diff and I believe I have addressed all of the issues you 
> pointed.
> 
> I also added the usability group. 
> 
> IMHO I don't think having this option is bad as I guess that the 
> "correct" icon size is depended to the monitor so maybe it is better to be 
> controlled by the user instead of having a fixed value :\
> 
> Marco Martin wrote:
> well, depending from the monitor means it should depend from the monitor 
> dpi, rather than the user (and yes, medium size was definitely too big, but i 
> don't think it grants an option to explicitly control this)

iconSizes are scaled with DPI, no?


- Kai Uwe


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97186
---


On Juli 8, 2016, 8:56 vorm., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated Juli 8, 2016, 8:56 vorm.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Marco Martin]

> On July 8, 2016, 7:21 a.m., Kai Uwe Broulik wrote:
> > Thanks for your patch!
> > 
> > However, I don't think there should be such an option. There's no other 
> > applet which has this and applets in the panel usually follow the panel 
> > size indefinitely. Perhaps add the "usability" group to have them decide.
> > 
> > Also, I don't think this single option deserves a separage page, it would 
> > fit just fine at the top of the "General" page imho.
> 
> John Salatas wrote:
> Thanks for your feedback! 
> 
> I updated the diff and I believe I have addressed all of the issues you 
> pointed.
> 
> I also added the usability group. 
> 
> IMHO I don't think having this option is bad as I guess that the 
> "correct" icon size is depended to the monitor so maybe it is better to be 
> controlled by the user instead of having a fixed value :\

well, depending from the monitor means it should depend from the monitor dpi, 
rather than the user (and yes, medium size was definitely too big, but i don't 
think it grants an option to explicitly control this)


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97186
---


On July 8, 2016, 8:56 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:56 a.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

> On July 8, 2016, 7:21 a.m., Kai Uwe Broulik wrote:
> > Thanks for your patch!
> > 
> > However, I don't think there should be such an option. There's no other 
> > applet which has this and applets in the panel usually follow the panel 
> > size indefinitely. Perhaps add the "usability" group to have them decide.
> > 
> > Also, I don't think this single option deserves a separage page, it would 
> > fit just fine at the top of the "General" page imho.

Thanks for your feedback! 

I updated the diff and I believe I have addressed all of the issues you pointed.

I also added the usability group. 

IMHO I don't think having this option is bad as I guess that the "correct" icon 
size is depended to the monitor so maybe it is better to be controlled by the 
user instead of having a fixed value :\


- John


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97186
---


On July 8, 2016, 8:41 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 8:41 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/
---

(Updated July 8, 2016, 8:41 a.m.)


Review request for Plasma.


Repository: plasma-workspace


Description
---

I didn't like the recent changes in systray icon size related to bug #364431, 
so I created this patch in order to make the systray's icon size user 
configurable


Diffs (updated)
-

  applets/systemtray/package/contents/code/IconSizeTools.js PRE-CREATION 
  applets/systemtray/package/contents/config/main.xml 65a7029 
  applets/systemtray/package/contents/ui/ConfigGeneral.qml 4fcfcf6 
  applets/systemtray/package/contents/ui/main.qml a66ea69 

Diff: https://git.reviewboard.kde.org/r/128400/diff/


Testing
---

Tested in KDE Neon Developer Stable (as of July 7, 2016)


Thanks,

John Salatas

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97191
---



I really don't want an option for this :(

- Marco Martin


On July 8, 2016, 4:29 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 4:29 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigIcons.qml PRE-CREATION 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Anthony Fieroni]

> On Юли 8, 2016, 9:57 преди обяд, Anthony Fieroni wrote:
> > About me, size of systray icons must depend on panel size. Same for space 
> > between items.
> 
> John Salatas wrote:
> Well... I guess it's just a matter of personal taste. I'm using a rather 
> high panel and the default icon size (units.iconSizes.medium) seems rather 
> big. I would prefer units.iconSizes.smallMedium instead. 
> 
> You can compare the two choices in the screenshot I uploaded in bug 
> #364431 (https://bugsfiles.kde.org/attachment.cgi?id=99911): the upper is the 
> default size (units.iconSizes.medium) and the lower which I prefer is 
> units.iconSizes.smallMedium

Yeah, i saw the comparison. I don't see any reason icons in systray to be 
treated different from any other. They must be scaled to systray size.


- Anthony


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97184
---


On Юли 8, 2016, 7:29 преди обяд, John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated Юли 8, 2016, 7:29 преди обяд)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigIcons.qml PRE-CREATION 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Kai Uwe Broulik]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97186
---



Thanks for your patch!

However, I don't think there should be such an option. There's no other applet 
which has this and applets in the panel usually follow the panel size 
indefinitely. Perhaps add the "usability" group to have them decide.

Also, I don't think this single option deserves a separage page, it would fit 
just fine at the top of the "General" page imho.


applets/systemtray/package/contents/ui/ConfigIcons.qml (lines 2 - 3)


This is not you



applets/systemtray/package/contents/ui/ConfigIcons.qml (line 31)


Whitespace.

You have lots of whitespace errors all over the place.



applets/systemtray/package/contents/ui/ConfigIcons.qml (line 38)


Isn't a Slider a better control for this? See FolderView icon size setting.



applets/systemtray/package/contents/ui/ConfigIcons.qml (line 67)


Use onActivated instead which is only emitted when the user himself 
explicitly changes the index, ie.
onActivated: cfg_iconSize = model[index].name

("index" is the parameter of the signal, currentIndex isn't updated at the 
time this signal is fired unfortunately)



applets/systemtray/package/contents/ui/main.qml (line 44)


You could just do units.iconSizes[plasmoid.configuration.iconSize]



applets/systemtray/package/contents/ui/main.qml 


Unrelated change


- Kai Uwe Broulik


On Juli 8, 2016, 4:29 vorm., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated Juli 8, 2016, 4:29 vorm.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigIcons.qml PRE-CREATION 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

> On July 8, 2016, 6:57 a.m., Anthony Fieroni wrote:
> > About me, size of systray icons must depend on panel size. Same for space 
> > between items.

Well... I guess it's just a matter of personal taste. I'm using a rather high 
panel and the default icon size (units.iconSizes.medium) seems rather big. I 
would prefer units.iconSizes.smallMedium instead. 

You can compare the two choices in the screenshot I uploaded in bug #364431 
(https://bugsfiles.kde.org/attachment.cgi?id=99911): the upper is the default 
size (units.iconSizes.medium) and the lower which I prefer is 
units.iconSizes.smallMedium


- John


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97184
---


On July 8, 2016, 4:29 a.m., John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated July 8, 2016, 4:29 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigIcons.qml PRE-CREATION 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 128400: Configuration option for System Tray's icon size

[Anthony Fieroni]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/#review97184
---



About me, size of systray icons must depend on panel size. Same for space 
between items.

- Anthony Fieroni


On Юли 8, 2016, 7:29 преди обяд, John Salatas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128400/
> ---
> 
> (Updated Юли 8, 2016, 7:29 преди обяд)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> I didn't like the recent changes in systray icon size related to bug #364431, 
> so I created this patch in order to make the systray's icon size user 
> configurable
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/config/main.xml 65a7029 
>   applets/systemtray/package/contents/ui/ConfigIcons.qml PRE-CREATION 
>   applets/systemtray/package/contents/ui/main.qml a66ea69 
> 
> Diff: https://git.reviewboard.kde.org/r/128400/diff/
> 
> 
> Testing
> ---
> 
> Tested in KDE Neon Developer Stable (as of July 7, 2016)
> 
> 
> Thanks,
> 
> John Salatas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 128400: Configuration option for System Tray's icon size

[John Salatas]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128400/
---

Review request for Plasma.


Repository: plasma-workspace


Description
---

I didn't like the recent changes in systray icon size related to bug #364431, 
so I created this patch in order to make the systray's icon size user 
configurable


Diffs
-

  applets/systemtray/package/contents/config/main.xml 65a7029 
  applets/systemtray/package/contents/ui/ConfigIcons.qml PRE-CREATION 
  applets/systemtray/package/contents/ui/main.qml a66ea69 

Diff: https://git.reviewboard.kde.org/r/128400/diff/


Testing
---

Tested in KDE Neon Developer Stable (as of July 7, 2016)


Thanks,

John Salatas

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel