D4845: Fix context menu button's icon size when on default DPI

2017-03-08 Thread Chris Holland
This revision was automatically updated to reflect the committed changes. Closed by commit R115:4d4867b89996: Fix context menu button's icon size when on default DPI (authored by Zren). REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4845?vs=

D4845: Fix context menu button's icon size when on default DPI

2017-03-06 Thread Albert Astals Cid
aacid resigned from this revision. aacid added a comment. This revision is now accepted and ready to land. Abstain REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D4845 To: Zren, subdiff, broulik, drosca Cc: aacid, davidedmundson, plasma-devel, prog

D4845: Fix context menu button's icon size when on default DPI

2017-03-06 Thread David Rosca
drosca accepted this revision. REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D4845 To: Zren, subdiff, broulik, drosca, aacid Cc: aacid, davidedmundson, plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D4845: Fix context menu button's icon size when on default DPI

2017-03-05 Thread Chris Holland
Zren updated this revision to Diff 12221. Zren added a comment. Fix merge conflict based on removal of x/y parameters in contextmenu.show() Change height from slider.height to units.iconSizes.small REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.

D4845: Fix context menu button's icon size when on default DPI

2017-03-05 Thread Chris Holland
Zren added a comment. Ah, the `contextMenu.show(x, y)` changed to `contextMenu.show()`. I've got to comment out `roundToIconSize: false` in the mute "button" to test now since that probably requires a new version of frameworks. I changed the `slider.height` to `Layout.preferredHeight:

D4845: Fix context menu button's icon size when on default DPI

2017-03-03 Thread David Rosca
drosca added a comment. > So we probably need to set Layout.preferredHeight: Math.max(slider.height, units.iconSizes.small) in the ToolButton? Actually, let's make this and also volume indicator icon (next to theslider) have size = units.iconSizes.small and remove the relative sizing by

D4845: Fix context menu button's icon size when on default DPI

2017-03-03 Thread Albert Astals Cid
aacid requested changes to this revision. aacid added a comment. This revision now requires changes to proceed. Patch does not seem to apply REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D4845 To: Zren, subdiff, drosca, broulik, aacid Cc: aacid, d

[Differential] [Commented On] D4845: Fix context menu button's icon size when on default DPI

2017-03-01 Thread Chris Holland
Zren added a comment. @davidedmundson: Yes. I usually test in plasmoidviewer since it's the same (except for emoji rendering ). I get the same effect because you're still trying to stuff a 16px icon into a 12px hole at 1x DPI (96). The origina

[Differential] [Commented On] D4845: Fix context menu button's icon size when on default DPI

2017-02-28 Thread David Edmundson
davidedmundson added a comment. @zren did you get that original problem in plasmashell as well as plasmoidviewer? The description doesn't make much sense because Plasmashell disables Qt's high DPI stuff. REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator

[Differential] [Commented On] D4845: Fix context menu button's icon size when on default DPI

2017-02-28 Thread Chris Holland
Zren added a comment. That's a good idea @broulik . Slider height for the Desktop themes I have installed (I used this script ): - air: 20px - Arctica: 24px - Arezzo: 24px - arquetype-dark: 24px - Bise: 24px - b

[Differential] [Updated] D4845: Fix context menu button's icon size when on default DPI

2017-02-28 Thread Kai Uwe Broulik
broulik added a comment. What about making ToolButton's icon size a minimum of small icon size no matter what? I don't see a case where you ever want to have it smaller. REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D4845 EMAIL PREFERENCES http

[Differential] [Updated] D4845: Fix context menu button's icon size when on default DPI

2017-02-28 Thread Chris Holland
Zren marked 3 inline comments as done. REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D4845 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: Zren, subdiff, drosca, broulik Cc: plasma-devel, progwolff, lesliezhai, al

[Differential] [Updated, 10 lines] D4845: Fix context menu button's icon size when on default DPI

2017-02-28 Thread Chris Holland
Zren updated this revision to Diff 11969. Zren added a comment. Remove `|| !parent.flat` REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4845?vs=11964&id=11969 REVISION DETAIL https://phabricator.kde.org/D4845 AFFECTED FILES applet/c

[Differential] [Accepted] D4845: Fix context menu button's icon size when on default DPI

2017-02-28 Thread Roman Gilg
subdiff accepted this revision. subdiff added a comment. This revision is now accepted and ready to land. Tested it and works well. You can omit the `!parent.flat` condition as @drosca said, since it's always false. REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabr

[Differential] [Commented On] D4845: Fix context menu button's icon size when on default DPI

2017-02-28 Thread Chris Holland
Zren added inline comments. INLINE COMMENTS > Zren wrote in ListItemBase.qml:122 > Not really. Just wanted to have consistent code with ToolButtonStyle, but I > already had to change `control` => `parent` so might as well remove it. > ToolButton is basically a `flat: true` Button so that part w

[Differential] [Commented On] D4845: Fix context menu button's icon size when on default DPI

2017-02-28 Thread Chris Holland
Zren added inline comments. INLINE COMMENTS > drosca wrote in ListItemBase.qml:122 > Do we really need this (we certainly don't need the `parent.flat` part)? It > doesn't seem to change the icon at all. Not really. Just wanted to have consistent code with ToolButtonStyle, but I already had to

[Differential] [Commented On] D4845: Fix context menu button's icon size when on default DPI

2017-02-28 Thread David Rosca
drosca added inline comments. INLINE COMMENTS > ListItemBase.qml:122 > +active: parent.hovered > +colorGroup: parent.hovered || !parent.flat ? > PlasmaCore.Theme.ButtonColorGroup : PlasmaCore.ColorScope.colorGroup > +

[Differential] [Updated] D4845: Fix context menu button's icon size when on default DPI

2017-02-28 Thread Chris Holland
Zren edited the summary of this revision. REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D4845 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: Zren, subdiff, drosca, broulik Cc: plasma-devel, progwolff, lesliezhai,

[Differential] [Request, 10 lines] D4845: Fix context menu button's icon size when on default DPI

2017-02-28 Thread Chris Holland
Zren created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY ToolButton pads the contexts of the label+icon based on the desktop theme's svg margins. Causing the icon to only be 12px, which is scaled down a