D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos added a comment. Things look bad when a bitmap icon appears in the tray... (e.g. Remmina) It seems `roundToIconSize` is necessary for bitmap icons. I think I am going to abandon this patch, as the necessary changes are simply too big: - all references to `marginHints` in the

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos added a comment. With this new version the tray icons scale exactly like the application icons on the left (the "jumps" between icon sizes happen at exactly the same time). INLINE COMMENTS > anthonyfieroni wrote in main.qml:42 > I see it has configuration for icon size ? What about to

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos updated this revision to Diff 27248. pgkos added a comment. Cancel out the enlargement of AbstractItem by the tasksRow's marginHints. REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10487?vs=27139=27248 BRANCH tray-icon-size-fix REVISION

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos added a comment. It seems the problem is not with SVG icons, but with the systemtray code. If I make the systemtray's Flow's marginHints zero, the padding is preserved correctly! No magic constant is needed anymore: //Do spacing with margins, to correctly compute the number of

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

davidedmundson added a comment. Thanks, that's much more the sort of thought process I'm looking for! > the tray icons are SVG icons, but the application shorcut icons are bitmaps. All application icons are SVGs and they both ultimately come from IconItem? I think we might find

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

anthonyfieroni added inline comments. INLINE COMMENTS > main.qml:42 > property bool vertical: plasmoid.formFactor == PlasmaCore.Types.Vertical > -property int itemSize: units.roundToIconSize(Math.min(Math.min(width, > height),

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos added a comment. There is something wrong and inconstent about SVG icon scaling. For example, if I put this line in systemtray ui main QML code instead: `property int itemSize: Math.min(width, height)` then the KDE Connect icon completely fills the height of the panel.

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

davidedmundson added a comment. The part I care about is deciding what the problem is, and understanding the code around it. The goal apparently was to match the application icon on the left, which means any patch needs to be able to describe the current padding that happens there, and

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

ngraham added a comment. Going up from the 0.71 magic number to the 0.85 magic number would make the clock bigger than it currently is, no? Can you supply a screenshot of how this works now at standard size, increased size, and witha high-DPI display? Since magic numbers are bad, and

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

davidedmundson requested changes to this revision. davidedmundson added a comment. This revision now requires changes to proceed. no. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D10487 To: pgkos, #plasma_workspaces, davidedmundson Cc: ngraham,

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos added a comment. I have returned to the previous version, as the version which used units.smallSpacing looked awful. Yes, it uses magic constant 0.85, but it really looks much better. I have patched the digital clock height too, so it perfectly matches the height of the tray

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos updated this revision to Diff 27139. pgkos added a comment. Fix the previous commit REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10487?vs=27138=27139 BRANCH tray-icon-size-fix REVISION DETAIL https://phabricator.kde.org/D10487

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos updated this revision to Diff 27138. pgkos added a comment. Return to previous version, which was better. REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10487?vs=27114=27138 BRANCH tray-icon-size-fix REVISION DETAIL

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos updated this revision to Diff 27114. pgkos added a comment. Use units.smallSpacing for padding instead of magic numbers. REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10487?vs=27105=27114 BRANCH tray-icon-size-fix REVISION DETAIL

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

ngraham added a comment. Padding is fine. We're just saying that: - You need to retain the default appearance for a default sized panel - You need to use programmatic padding values rather than magic numbers REPOSITORY R120 Plasma Workspace REVISION DETAIL

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos added a comment. I have tried and tested this diff without any padding, and trust me, it looks very bad - **I would rather have this patch not accepted** - than not to add any padding. Particularly, the tray icons look extremely bad when there is no padding - I suppose this is

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

davidedmundson added a comment. Yeah, the clock code isn't something I'd accept either. > But with your attitude it will be difficult to justify anything. Plasma has a predefined margin size smallSpacing. If you're using something else, it probably isnt' going to end up consistent.

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

ngraham added a comment. In D10487#205827 , @pgkos wrote: > I could argue that the numbers come from Material Design guidelines - it recommends between 10% to 20% of padding

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos added a comment. I could argue that the numbers come from Material Design guidelines - it recommends between 10% to 20% of padding around icons. But with your attitude it will be difficult to justify anything.

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos added a comment. So maybe you tell me where this comes from? https://cgit.kde.org/plasma-workspace.git/commit/applets/digital-clock/package/contents/ui/DigitalClock.qml?id=90ea27f49c84f0dffbbf79b29eaa14c57f31a24d REPOSITORY R120 Plasma Workspace REVISION DETAIL

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

davidedmundson requested changes to this revision. davidedmundson added a comment. This revision now requires changes to proceed. I'm not merging a patch that just adds random numbers that you happen to think look good. It's not unification if you can't justify where the numbers come

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos added a comment. I have tested it with the panel in both horizontal and vertical orientations, and 0.85 looks optimal for me. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D10487 To: pgkos, #plasma_workspaces, davidedmundson Cc: ngraham,

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos updated this revision to Diff 27105. pgkos added a comment. Squashed the commits REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10487?vs=27103=27105 BRANCH tray-icon-size-fix REVISION DETAIL https://phabricator.kde.org/D10487 AFFECTED

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos updated this revision to Diff 27103. pgkos added a comment. Make the padding of all the panel icons the same CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10487?vs=27095=27103 BRANCH tray-icon-size-fix REVISION DETAIL https://phabricator.kde.org/D10487 AFFECTED FILES

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

davidedmundson requested changes to this revision. davidedmundson added a comment. This revision now requires changes to proceed. > The 0.8 is used to create a small padding, so the icons never touch the panel's borders. If the goal of the patch is to make the icon sizes the same as the

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

ngraham added a comment. I fear this will generate the exact same types of user complaints that the huge clock text did before we gave it more vertical padding: people will say that the icons are too tall and don't have enough padding. Let's try not changing the appearance and vertical

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos added a comment. This patch also enables the user to configure the tray icon's size limit using System Settings -> Icons -> Advanced -> Panel REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D10487 To: pgkos, #plasma_workspaces Cc: ngraham,

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos added a comment. The 0.8 is used to create a small padding, so the icons never touch the panel's borders. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D10487 To: pgkos, #plasma_workspaces Cc: ngraham, davidedmundson, plasma-devel, ZrenBot,

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos edited the summary of this revision. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D10487 To: pgkos, #plasma_workspaces Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos updated this revision to Diff 27095. pgkos added a comment. Fix tray icon scaling REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10487?vs=27089=27095 BRANCH tray-icon-size-fix REVISION DETAIL https://phabricator.kde.org/D10487

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos edited the test plan for this revision. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D10487 To: pgkos, #plasma_workspaces Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

ngraham added a comment. Thanks for the patch! 1. Please follow the formatting guidelines in https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch 2. Please update the Test Plan section with evidence of testing. A pair of before-and-after screenshots is always

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

davidedmundson added a comment. Which application icons, and where are they set to 0.8? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D10487 To: pgkos, #plasma_workspaces Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed,

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

pgkos created this revision. pgkos added a reviewer: Plasma: Workspaces. pgkos added a project: Plasma: Workspaces. Restricted Application edited projects, added Plasma; removed Plasma: Workspaces. Restricted Application added a subscriber: plasma-devel. pgkos requested review of this revision.