fix layout size hints for button labels

Summary:
icon sizes and label placements tested correct in  several scenarios

* control.icon.width/height is used as maximum size of the icon
* if the button is smaller icons always scale down
* icons stay ccentered regardless of button size when there is no text

Test Plan:
fixes icon sizes without workarounds like D27260
{F8108705}

Reviewers: #plasma, davidedmundson

Reviewed By: #plasma, davidedmundson

Subscribers: davidedmundson, kde-frameworks-devel

Tags: #frameworks

Differential Revision: https://phabricator.kde.org/D27460

src/declarativeimports/plasmacomponents3/Button.qml

@@ -31,6 +31,8 @@ T.Button {
                             contentItem.implicitWidth + leftPadding + rightPadding)
     implicitHeight: Math.max(background.implicitHeight, contentItem.implicitHeight + topPadding + bottomPadding)
 
+    Layout.minimumWidth: contentItem.implicitWidth + leftPadding + rightPadding
+
     leftPadding: surfaceNormal.margins.left
     topPadding: surfaceNormal.margins.top
     rightPadding: surfaceNormal.margins.right
@@ -42,28 +44,41 @@ T.Button {
     Kirigami.MnemonicData.controlType: Kirigami.MnemonicData.SecondaryControl
     Kirigami.MnemonicData.label: control.text
 
-    contentItem: RowLayout {
+    PlasmaCore.ColorScope.inherit: flat
+    PlasmaCore.ColorScope.colorGroup: flat ? parent.PlasmaCore.ColorScope.colorGroup : PlasmaCore.Theme.ButtonColorGroup
+
+    contentItem: GridLayout {
+        columns: control.display == T.AbstractButton.TextBesideIcon ? 2 : 1
         PlasmaCore.IconItem {
             id: icon
 
             Layout.alignment: Qt.AlignCenter
-            Layout.preferredWidth: control.icon.width > 0 ? control.icon.width : units.iconSizes.small
-            Layout.preferredHeight: control.icon.height > 0 ? control.icon.height : units.iconSizes.small
+
+            Layout.fillWidth: control.display === T.Button.TextUnderIcon
+            Layout.fillHeight: control.display !== T.Button.TextUnderIcon
+
+            Layout.minimumWidth: units.iconSizes.tiny
+            Layout.minimumHeight: units.iconSizes.tiny
+            Layout.maximumWidth: control.icon.width > 0 ? control.icon.width : units.iconSizes.small
+            Layout.maximumHeight: control.icon.height > 0 ? control.icon.height : units.iconSizes.small
+
+            implicitWidth: Layout.maximumWidth
+            implicitHeight: Layout.maximumHeight
 
             colorGroup: PlasmaCore.Theme.ButtonColorGroup
-            visible: source.length > 0
+            visible: source.length > 0 && control.display !== T.Button.TextOnly
             source: control.icon ? (control.icon.name || control.icon.source) : ""
-            status: buttonSvg.hasElement("hint-focus-highlighted-background") && control.activeFocus && !control.pressed && !control.checked ? PlasmaCore.Svg.Selected : PlasmaCore.Svg.Normal
+            status: !control.flat && buttonSvg.hasElement("hint-focus-highlighted-background") && control.activeFocus && !control.pressed && !control.checked ? PlasmaCore.Svg.Selected : PlasmaCore.Svg.Normal
         }
         Label {
+            id: label
             Layout.fillWidth: true
-            Layout.fillHeight: true
-            visible: text.length > 0
+            visible: text.length > 0 && control.display !== T.Button.IconOnly
             text: control.Kirigami.MnemonicData.richTextLabel
             font: control.font
             opacity: enabled || control.highlighted || control.checked ? 1 : 0.4
             color: buttonSvg.hasElement("hint-focus-highlighted-background") && control.activeFocus && !control.down ? theme.highlightedTextColor : theme.buttonTextColor
-            horizontalAlignment: Text.AlignHCenter
+            horizontalAlignment: control.display !== T.Button.TextUnderIcon && icon.visible ? Text.AlignLeft : Text.AlignHCenter
             verticalAlignment: Text.AlignVCenter
             elide: Text.ElideRight
         }
@@ -71,8 +86,8 @@ T.Button {
 
     background: Item {
         //retrocompatibility with old controls
-        implicitWidth: Math.floor(units.gridUnit * 1.6) + Math.floor(units.gridUnit * 1.6) % 2
-        implicitHeight: implicitWidth
+        implicitWidth: label.visible ? units.gridUnit * 6 : implicitHeight
+        implicitHeight: Math.floor(units.gridUnit * 1.6) + Math.floor(units.gridUnit * 1.6) % 2
         Private.ButtonShadow {
             anchors.fill: parent
             visible: (!control.flat || control.hovered) && (!control.pressed || !control.checked)

src/declarativeimports/plasmacomponents3/TabButton.qml

@@ -46,18 +46,26 @@ T.TabButton {
             id: icon
 
             Layout.alignment: Qt.AlignCenter
-            Layout.preferredWidth: control.icon.width > 0 ? control.icon.width : units.iconSizes.smallMedium
-            Layout.preferredHeight: control.icon.height > 0 ? control.icon.height : units.iconSizes.smallMedium
 
-            colorGroup: PlasmaCore.Theme.ButtonColorGroup
-            visible: source.length > 0
+            Layout.fillWidth: control.display === T.AbstractButton.TextUnderIcon
+            Layout.fillHeight: control.display !== T.AbstractButton.TextUnderIcon
+
+            Layout.minimumWidth: units.iconSizes.tiny
+            Layout.minimumHeight: units.iconSizes.tiny
+            Layout.maximumWidth: control.icon.width > 0 ? control.icon.width : units.iconSizes.smallMedium
+            Layout.maximumHeight: control.icon.height > 0 ? control.icon.height : units.iconSizes.smallMedium
+
+            implicitWidth: Layout.maximumWidth
+            implicitHeight: Layout.maximumHeight
+
+            colorGroup: control.PlasmaCore.ColorScope.colorGroup
+            visible: source.length > 0 && control.display !== T.AbstractButton.TextOnly
             source: control.icon ? (control.icon.name || control.icon.source) : ""
-            status: control.activeFocus && !control.pressed && !control.checked ? PlasmaCore.Svg.Selected : PlasmaCore.Svg.Normal
         }
         Label {
+            id: label
             Layout.fillWidth: true
-            Layout.fillHeight: true
-            visible: text.length > 0
+            visible: text.length > 0 && control.display !== T.AbstractButton.IconOnly
             text: control.Kirigami.MnemonicData.richTextLabel
             font: control.font
             opacity: enabled || control.highlighted || control.checked ? 1 : 0.4

src/declarativeimports/plasmacomponents3/TextField.qml

@@ -32,7 +32,7 @@ T.TextField {
     implicitWidth: Math.max(units.gridUnit * 8,
                             placeholderText ? placeholder.implicitWidth + leftPadding + rightPadding : 0)
                             || contentWidth + leftPadding + rightPadding
-    implicitHeight: Math.floor(units.gridUnit * 1.6) + Math.floor(units.gridUnit * 1.6) % 2
+    implicitHeight: background.implicitHeight
 
     padding: 6
 
@@ -100,6 +100,13 @@ T.TextField {
     }
 
     background: Item {
+        implicitHeight: Math.max(Math.floor(metrics.height * 1.6) + Math.floor(metrics.height * 1.6) % 2,
+                                 metrics.height + base.margins.top + base.margins.bottom)
+        TextMetrics {
+            id: metrics
+            text: "M"
+            font: control.font
+        }
         Private.TextFieldFocus {
             state: control.activeFocus ? "focus" : (control.hovered ? "hover" : "hidden")
             anchors.fill: parent

src/declarativeimports/plasmacomponents3/ToolButton.qml

@@ -45,27 +45,41 @@ T.ToolButton {
 
     flat: true
 
+    PlasmaCore.ColorScope.inherit: flat
+    PlasmaCore.ColorScope.colorGroup: flat ? parent.PlasmaCore.ColorScope.colorGroup : PlasmaCore.Theme.ButtonColorGroup
+    
     contentItem: GridLayout {
         columns: control.display == T.AbstractButton.TextBesideIcon ? 2 : 1
+
         PlasmaCore.IconItem {
             id: icon
 
             Layout.alignment: Qt.AlignCenter
-            Layout.preferredWidth: control.icon.width > 0 ? control.icon.width : units.iconSizes.smallMedium
-            Layout.preferredHeight: control.icon.height > 0 ? control.icon.height : units.iconSizes.smallMedium
 
-            colorGroup: PlasmaCore.Theme.ButtonColorGroup
-            visible: source.length > 0
+            Layout.fillWidth: control.display === T.AbstractButton.TextUnderIcon
+            Layout.fillHeight: control.display !== T.AbstractButton.TextUnderIcon
+
+            Layout.minimumWidth: units.iconSizes.tiny
+            Layout.minimumHeight: units.iconSizes.tiny
+            Layout.maximumWidth: control.icon.width > 0 ? control.icon.width : units.iconSizes.smallMedium
+            Layout.maximumHeight: control.icon.height > 0 ? control.icon.height : units.iconSizes.smallMedium
+
+            implicitWidth: Layout.maximumWidth
+            implicitHeight: Layout.maximumHeight
+
+            colorGroup: control.PlasmaCore.ColorScope.colorGroup
+            visible: source.length > 0 && control.display !== T.AbstractButton.TextOnly
             source: control.icon ? (control.icon.name || control.icon.source) : ""
-            status: control.activeFocus && !control.pressed && !control.checked ? PlasmaCore.Svg.Selected : PlasmaCore.Svg.Normal
+            status: !control.flat && control.activeFocus && !control.pressed && !control.checked ? PlasmaCore.Svg.Selected : PlasmaCore.Svg.Normal
         }
         
         Label {
+            id: label
             Layout.fillWidth: true
-            visible: text.length > 0
+            visible: text.length > 0 && control.display !== T.AbstractButton.IconOnly
             text: control.Kirigami.MnemonicData.richTextLabel
             font: control.font
-            opacity: enabled || control.highlighted || control.checked ? 1 : 0.4
+            opacity: width > 0 ? (enabled || control.highlighted || control.checked ? 1 : 0.4) : 0
             color: (control.hovered || !control.flat) && buttonsurfaceChecker.usedPrefix != "toolbutton-hover" ? theme.buttonTextColor : PlasmaCore.ColorScope.textColor
             horizontalAlignment: Text.AlignHCenter
             verticalAlignment: Text.AlignVCenter

tests/components/button3.qml

@@ -87,7 +87,42 @@ ComponentBase {
         RowLayout {
             PlasmaComponents.Button {
                 text: "AA"
-//                 implicitWidth: minimumWidth FIXME, there is no equivalent?
+                implicitWidth: Layout.minimumWidth
+            }
+            PlasmaComponents.Button {
+                icon.name: "application-menu"
+                text: "AA"
+                implicitWidth: Layout.minimumWidth
+            }
+            PlasmaComponents.Button {
+                icon.name: "application-menu"
+                implicitWidth: Layout.minimumWidth
+            }
+        }
+
+        PlasmaComponents.Label {
+            text: "Display property"
+        }
+        RowLayout {
+            PlasmaComponents.Button {
+                icon.name: "application-menu"
+                text: "Icon Only"
+                display: PlasmaComponents.Button.IconOnly
+            }
+            PlasmaComponents.Button {
+                icon.name: "application-menu"
+                text: "Text Beside Icon"
+                display: PlasmaComponents.Button.TextBesideIcon
+            }
+            PlasmaComponents.Button {
+                icon.name: "application-menu"
+                text: "Text Under Icon"
+                display: PlasmaComponents.Button.TextUnderIcon
+            }
+            PlasmaComponents.Button {
+                icon.name: "application-menu"
+                text: "Text Only"
+                display: PlasmaComponents.Button.TextOnly
             }
         }
 

tests/components/tabbar3.qml

@@ -0,0 +1,35 @@
+import QtQuick 2.0
+
+import org.kde.plasma.core 2.0 as PlasmaCore
+
+import org.kde.plasma.components 3.0 as PlasmaComponents
+
+ComponentBase {
+    PlasmaComponents.TabBar {
+        anchors.fill: parent
+        anchors.margins: 20
+        spacing: 20
+
+        PlasmaComponents.TabButton {
+            icon.name: "application-menu"
+            text: "Icon Only"
+            display: PlasmaComponents.TabButton.IconOnly
+        }
+        PlasmaComponents.TabButton {
+            icon.name: "application-menu"
+            text: "Text Beside Icon"
+            display: PlasmaComponents.TabButton.TextBesideIcon
+        }
+        PlasmaComponents.TabButton {
+            icon.name: "application-menu"
+            text: "Text Under Icon"
+            display: PlasmaComponents.TabButton.TextUnderIcon
+        }
+        PlasmaComponents.TabButton {
+            icon.name: "application-menu"
+            text: "Text Only"
+            display: PlasmaComponents.TabButton.TextOnly
+        }
+    }
+}
+

tests/components/toolbutton3.qml

@@ -1,40 +1,84 @@
 import QtQuick 2.0
+import QtQuick.Layouts 1.4
 
 import org.kde.plasma.core 2.0 as PlasmaCore
 
 import org.kde.plasma.components 3.0 as PlasmaComponents
 
 ComponentBase {
-    Flow {
+    ColumnLayout {
         anchors.fill: parent
         anchors.margins: 20
-        spacing: 20
+        Flow {
+            Layout.fillWidth: true
+            Layout.fillHeight: true
+            spacing: 20
 
-        PlasmaComponents.ToolButton {
-            icon.name: "list-remove"
-            text: "test"
-            flat: true
+            PlasmaComponents.ToolButton {
+                icon.name: "list-remove"
+                text: "test"
+                flat: true
+            }
+            PlasmaComponents.ToolButton {
+                icon.name: "list-remove"
+                flat: true
+            }
+            PlasmaComponents.ToolButton {
+                text: "test"
+                flat: true
+            }
+            PlasmaComponents.ToolButton {
+                icon.name: "list-remove"
+                text: "test"
+                flat: false
+            }
+            PlasmaComponents.ToolButton {
+                icon.name: "list-remove"
+                flat: false
+            }
+            PlasmaComponents.ToolButton {
+                text: "test"
+                flat: false
+            }
+            PlasmaComponents.ToolButton {
+                icon.name: "application-menu"
+                text: "Icon Only"
+                display: PlasmaComponents.ToolButton.IconOnly
+            }
+            PlasmaComponents.ToolButton {
+                icon.name: "application-menu"
+                text: "Text Beside Icon"
+                display: PlasmaComponents.ToolButton.TextBesideIcon
+            }
+            PlasmaComponents.ToolButton {
+                icon.name: "application-menu"
+                text: "Text Under Icon"
+                display: PlasmaComponents.ToolButton.TextUnderIcon
+            }
+            PlasmaComponents.ToolButton {
+                icon.name: "application-menu"
+                text: "Text Only"
+                display: PlasmaComponents.ToolButton.TextOnly
+            }
         }
-        PlasmaComponents.ToolButton {
-            icon.name: "list-remove"
-            flat: true
-        }
-        PlasmaComponents.ToolButton {
-            text: "test"
-            flat: true
-        }
-        PlasmaComponents.ToolButton {
-            icon.name: "list-remove"
-            text: "test"
-            flat: false
-        }
-        PlasmaComponents.ToolButton {
-            icon.name: "list-remove"
-            flat: false
-        }
-        PlasmaComponents.ToolButton {
-            text: "test"
-            flat: false
+        RowLayout {
+            Layout.fillWidth: true
+            PlasmaComponents.Label {
+                Layout.fillWidth: true
+                text: "They should always be square:"
+            }
+            PlasmaComponents.ToolButton {
+                icon.name: "list-remove"
+            }
+            PlasmaComponents.ToolButton {
+                icon.name: "list-remove"
+            }
+            PlasmaComponents.ToolButton {
+                icon.name: "list-remove"
+            }
+            PlasmaComponents.ToolButton {
+                icon.name: "list-remove"
+            }
         }
     }
 }