From 4285a1ff09f9c18c0f15520b81c86d01557f72df Mon Sep 17 00:00:00 2001 From: Sebastian Gottfried Date: Wed, 19 Dec 2012 15:51:55 +0100 Subject: [PATCH] Plasma QML components: fix internal anchoring in Button and ToolButton There is a bug in the tool button and button components resulting in layout breakage if one clears and sets the text property of the component when not visible, see the attached screenshot. I have tried to solve the issue without changing the existing anchoring system, but without success. The only working solution was to put the icon and the label item into Row item. That way I was able to fix the bug and even get rid of the ugly explicit non-declarative anchor assignments. I have also removed the preferredWidth property of the label item, that one always evaluated to paintedWidth, anyway. REVIEW: 107813 --- .../plasmacomponents/qml/Button.qml | 33 +++++-------------- .../plasmacomponents/qml/ToolButton.qml | 33 +++++-------------- 2 files changed, 17 insertions(+), 49 deletions(-) diff --git a/declarativeimports/plasmacomponents/qml/Button.qml b/declarativeimports/plasmacomponents/qml/Button.qml index 5bfb8d74e..1168e6a7f 100644 --- a/declarativeimports/plasmacomponents/qml/Button.qml +++ b/declarativeimports/plasmacomponents/qml/Button.qml @@ -94,7 +94,7 @@ Item { /** * Smallest width this button can be to show all the contents */ - property real minimumWidth: icon.width + label.preferredWidth + surfaceNormal.margins.left + surfaceNormal.margins.right + ((icon.valid) ? surfaceNormal.margins.left : 0) + property real minimumWidth: icon.width + label.paintedWidth + surfaceNormal.margins.left + surfaceNormal.margins.right + ((icon.valid) ? surfaceNormal.margins.left : 0) /** * Smallest height this button can be to show all the contents @@ -191,9 +191,10 @@ Item { opacity: 0 } - Item { + Row { id: buttonContent state: (internal.userPressed || checked) ? "pressed" : "normal" + spacing: icon.valid ? surfaceNormal.margins.left : 0 states: [ State { name: "normal" }, @@ -229,35 +230,17 @@ Item { PlasmaCore.IconItem { id: icon - - anchors { - verticalCenter: parent.verticalCenter - left: label.text.length > 0 ? parent.left : undefined - horizontalCenter: label.text.length > 0 ? undefined : parent.horizontalCenter - } + anchors.verticalCenter: parent.verticalCenter + width: valid? parent.height: 0 + height: width active: shadow.hasOverState && mouse.containsMouse - height: parent.height - width: parent.height } Text { id: label - //FIXME: why this is needed? - onTextChanged: { - icon.anchors.horizontalCenter = label.text.length > 0 ? undefined : icon.parent.horizontalCenter - icon.anchors.left = label.text.length > 0 ? icon.parent.left : undefined - } - - property int preferredWidth: button.width < button.implicitWidth ? paintedWidth: paintedWidth - - anchors { - top: parent.top - bottom: parent.bottom - right: parent.right - left: icon.valid ? icon.right : parent.left - leftMargin: icon.valid ? parent.anchors.leftMargin : 0 - } + width: parent.width - icon.width - parent.spacing + height: parent.height font.capitalization: theme.defaultFont.capitalization font.family: theme.defaultFont.family diff --git a/declarativeimports/plasmacomponents/qml/ToolButton.qml b/declarativeimports/plasmacomponents/qml/ToolButton.qml index e5eb2bbba..bd6af7df2 100644 --- a/declarativeimports/plasmacomponents/qml/ToolButton.qml +++ b/declarativeimports/plasmacomponents/qml/ToolButton.qml @@ -87,7 +87,7 @@ Item { //icon + label + left margin + right margin + spacing between icon and text //here it assumesleft margin = right top = bottom, why? // because the right and bottom margins can be disabled, so they would return 0, but their actual size is still needed for size hints - property real minimumWidth: theme.smallIconSize + label.preferredWidth + delegate.margins.left + delegate.margins.left + ((icon.valid) ? delegate.margins.left : 0) + property real minimumWidth: theme.smallIconSize + label.paintedWidth + delegate.margins.left + delegate.margins.left + ((icon.valid) ? delegate.margins.left : 0) /** * The smallest height this button can be to show all the contents @@ -339,7 +339,7 @@ Item { } } - Item { + Row { anchors { fill: parent leftMargin: delegate.margins.left @@ -348,37 +348,22 @@ Item { bottomMargin: delegate.margins.bottom } + spacing: icon.valid ? delegate.margins.left : 0 + PlasmaCore.IconItem { id: icon - - anchors { - verticalCenter: parent.verticalCenter - left: label.text ? parent.left : undefined - horizontalCenter: label.text ? undefined : parent.horizontalCenter - } - height: parent.height - width: parent.height + anchors.verticalCenter: parent.verticalCenter + width: valid ? parent.height: 0 + height: width active: delegate.item.hasOverState && mouse.containsMouse } Text { id: label - //FIXME: why this is needed? - onPaintedWidthChanged: { - icon.anchors.horizontalCenter = label.paintedWidth > 0 ? undefined : icon.parent.horizontalCenter - icon.anchors.left = label.paintedWidth > 0 ? icon.parent.left : undefined - } + width: parent.width - icon.width - parent.spacing + height: parent.height - property int preferredWidth: button.width < button.implicitWidth ? paintedWidth: paintedWidth - - anchors { - top: parent.top - bottom: parent.bottom - left: icon.valid ? icon.right : parent.left - leftMargin: icon.valid ? delegate.margins.left : 0 - right: parent.right - } font.capitalization: theme.defaultFont.capitalization font.family: theme.defaultFont.family font.italic: theme.defaultFont.italic