Fix DialogButtonBox content size calculation
Some history: -f1f884d3
worked around an issue in DialogButtonBox. -c2fd8f7d
fixed it by using contentWidth; i.e. the implicit width of the contentItem. It caused QTBUG-72372. - I tried to fix QTBUG-72372 with6476de0b
, but created (or exposed) QTBUG-73860. The problem in QTBUG-73860 can be seen with the following example: Dialog { id: dialog visible: true standardButtons: Dialog.Ok } The single 'Ok' button here will go outside of the dialog. The underlying issue can be seen by looking into DialogButtonBox.qml: implicitWidth: Math.max(implicitBackgroundWidth + leftInset + rightInset, (control.count === 1 ? contentWidth * 2 : contentWidth) + leftPadding + rightPadding) implicitHeight: Math.max(implicitBackgroundHeight + topInset + bottomInset, contentHeight + topPadding + bottomPadding) The implicit width of the box in this case is contentWidth * 2 (there is one button, so control.count === 1). This should result in the button taking half the width of the box and being aligned to the right: alignment: count === 1 ? Qt.AlignRight : undefined ... delegate: Button { width: control.count === 1 ? control.availableWidth / 2 : undefined } What actually happens is that the contentItem (ListView) is temporarily 0 until it gets its final size of 100. However, QQuickDialogButtonBox doesn't respond to this change in the ListView's contentWidth. This problem can be fixed by returning to c2fd8f7d's resizeContent() implementation, which uses contentWidth. Then, there is a second issue: Dialog { id: dialog visible: true standardButtons: Dialog.Ok width: 300 } The button here is also positioned outside of the box. The problem is that the contentWidth is based on implicitContentWidth: QQuickContainerPrivate::updateContentWidth() { // ... contentWidth = implicitContentWidth; // ... } implicitContentWidth is calculated by calling getContentWidth(): void QQuickControlPrivate::updateImplicitContentWidth() { // ... implicitContentWidth = getContentWidth(); // ... } In the case of horizontal alignment, QQuickDialogButtonBoxPrivate::getContentWidth() uses the implicit width of the largest button: for (int i = 0; i < count; ++i) { QQuickItem *item = q->itemAt(i); if (item) { totalWidth += item->implicitWidth(); maxWidth = qMax(maxWidth, item->implicitWidth()); } } // ... if ((alignment & Qt::AlignHorizontal_Mask) == 0) totalWidth = qMax(totalWidth, count * maxWidth + totalSpacing); The Default style button has an implicitWidth of 100. The DialogButtonBox in the example above is 300 pixels wide, so the button should be 150, and it is, thanks to its width binding. However, the DialogButtonBox uses contentWidth to size its contentItem (ListView), and the contentWidth is, as mentioned, 100: the implicit width of the button. So, the button ends up hanging over the side of the box, as it's larger than the box thinks it is. This problem is fixed by setting DialogButtonBox's contentWidth to the contentWidth of the contentItem (ListView). This makes DialogButtonBox use the explicit widths of the buttons rather than their implicit widths. Since the contentWidth is no longer implicit, we must also change any use of contentWidth in the implicitWidth binding to implicitContentWidth. While writing auto tests for this, they caught an issue where contentWidth wasn't updated, so now we call resizeContent() in QQuickContainer::setContentWidth(). Change-Id: I99ffda21b47aeb14d4382e453e87c4312f343a1c Fixes: QTBUG-72886 Fixes: QTBUG-73860 Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
This commit is contained in:
parent
9a7c88bf7c
commit
8b78d9cea3
|
@ -41,9 +41,10 @@ T.DialogButtonBox {
|
|||
id: control
|
||||
|
||||
implicitWidth: Math.max(implicitBackgroundWidth + leftInset + rightInset,
|
||||
(control.count === 1 ? contentWidth * 2 : contentWidth) + leftPadding + rightPadding)
|
||||
(control.count === 1 ? implicitContentWidth * 2 : implicitContentWidth) + leftPadding + rightPadding)
|
||||
implicitHeight: Math.max(implicitBackgroundHeight + topInset + bottomInset,
|
||||
contentHeight + topPadding + bottomPadding)
|
||||
implicitContentHeight + topPadding + bottomPadding)
|
||||
contentWidth: contentItem.contentWidth
|
||||
|
||||
spacing: 1
|
||||
padding: 12
|
||||
|
|
|
@ -43,9 +43,10 @@ T.DialogButtonBox {
|
|||
id: control
|
||||
|
||||
implicitWidth: Math.max(implicitBackgroundWidth + leftInset + rightInset,
|
||||
(control.count === 1 ? contentWidth * 2 : contentWidth) + leftPadding + rightPadding)
|
||||
(control.count === 1 ? implicitContentWidth * 2 : implicitContentWidth) + leftPadding + rightPadding)
|
||||
implicitHeight: Math.max(implicitBackgroundHeight + topInset + bottomInset,
|
||||
contentHeight + topPadding + bottomPadding)
|
||||
implicitContentHeight + topPadding + bottomPadding)
|
||||
contentWidth: contentItem.contentWidth
|
||||
|
||||
spacing: 4
|
||||
padding: 24
|
||||
|
|
|
@ -793,6 +793,7 @@ void QQuickContainer::setContentWidth(qreal width)
|
|||
return;
|
||||
|
||||
d->contentWidth = width;
|
||||
d->resizeContent();
|
||||
emit contentWidthChanged();
|
||||
}
|
||||
|
||||
|
@ -832,6 +833,7 @@ void QQuickContainer::setContentHeight(qreal height)
|
|||
return;
|
||||
|
||||
d->contentHeight = height;
|
||||
d->resizeContent();
|
||||
emit contentHeightChanged();
|
||||
}
|
||||
|
||||
|
|
|
@ -241,11 +241,8 @@ void QQuickDialogButtonBoxPrivate::resizeContent()
|
|||
return;
|
||||
|
||||
QRectF geometry = q->boundingRect().adjusted(q->leftPadding(), q->topPadding(), -q->rightPadding(), -q->bottomPadding());
|
||||
if (alignment != 0) {
|
||||
qreal cw = (alignment & Qt::AlignHorizontal_Mask) == 0 ? q->availableWidth() : contentItem->property("contentWidth").toReal();
|
||||
qreal ch = (alignment & Qt::AlignVertical_Mask) == 0 ? q->availableHeight() : contentItem->property("contentHeight").toReal();
|
||||
geometry = alignedRect(q->isMirrored() ? Qt::RightToLeft : Qt::LeftToRight, alignment, QSizeF(cw, ch), geometry);
|
||||
}
|
||||
if (alignment != 0)
|
||||
geometry = alignedRect(q->isMirrored() ? Qt::RightToLeft : Qt::LeftToRight, alignment, QSizeF(contentWidth, contentHeight), geometry);
|
||||
|
||||
contentItem->setPosition(geometry.topLeft());
|
||||
contentItem->setSize(geometry.size());
|
||||
|
|
|
@ -54,8 +54,8 @@ import QtQuick.Controls 2.12
|
|||
|
||||
TestCase {
|
||||
id: testCase
|
||||
width: 200
|
||||
height: 200
|
||||
width: 600
|
||||
height: 400
|
||||
visible: true
|
||||
when: windowShown
|
||||
name: "DialogButtonBox"
|
||||
|
@ -292,4 +292,136 @@ TestCase {
|
|||
verify(buttonPosInBox.x >= 0)
|
||||
verify(buttonPosInBox.x + button.width < control.width)
|
||||
}
|
||||
|
||||
Component {
|
||||
id: dialogComponent
|
||||
// Based on the Default style, where a single button fills
|
||||
// half the dialog's width and is aligned to the right.
|
||||
Dialog {
|
||||
id: control
|
||||
standardButtons: Dialog.Ok
|
||||
visible: true
|
||||
|
||||
footer: DialogButtonBox {
|
||||
id: box
|
||||
visible: count > 0
|
||||
alignment: count === 1 ? Qt.AlignRight : undefined
|
||||
|
||||
implicitWidth: Math.max(implicitBackgroundWidth + leftInset + rightInset,
|
||||
(count === 1 ? implicitContentWidth * 2 : implicitContentWidth) + leftPadding + rightPadding)
|
||||
implicitHeight: Math.max(implicitBackgroundHeight + topInset + bottomInset,
|
||||
implicitContentHeight + topPadding + bottomPadding)
|
||||
contentWidth: contentItem.contentWidth
|
||||
|
||||
delegate: Button {
|
||||
width: box.count === 1 ? box.availableWidth / 2 : undefined
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// QTBUG-73860
|
||||
function test_oneButtonAlignedRightInImplicitWidthBox() {
|
||||
var dialog = createTemporaryObject(dialogComponent, testCase)
|
||||
verify(dialog)
|
||||
|
||||
var box = dialog.footer
|
||||
var listView = box.contentItem
|
||||
waitForRendering(listView)
|
||||
|
||||
var button = box.itemAt(0)
|
||||
verify(button)
|
||||
|
||||
// The button should never go outside of the box.
|
||||
var buttonPosInBox = button.mapToItem(box, 0, 0)
|
||||
verify(buttonPosInBox.x >= 0, "Expected button to be inside left edge "
|
||||
+ "of DialogButtonBox, but it's " + buttonPosInBox.x)
|
||||
verify(buttonPosInBox.x + button.width <= box.width, "Expected button to be inside right edge "
|
||||
+ "of DialogButtonBox (" + box.width + "), but it's " + (buttonPosInBox.x + button.width))
|
||||
compare(box.width, dialog.width)
|
||||
// There's a single button and we align it to the right.
|
||||
compare(box.contentItem.width, button.width)
|
||||
compare(box.contentItem.x, box.width - box.rightPadding - box.contentItem.width)
|
||||
}
|
||||
|
||||
Component {
|
||||
id: customButtonBox
|
||||
|
||||
DialogButtonBox {
|
||||
objectName: "customButtonBox"
|
||||
alignment: Qt.AlignRight
|
||||
|
||||
property alias okButton: okButton
|
||||
|
||||
Button {
|
||||
id: okButton
|
||||
text: "OK"
|
||||
|
||||
DialogButtonBox.buttonRole: DialogButtonBox.AcceptRole
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// QTBUG-72886
|
||||
function test_oneCustomButtonChangeText() {
|
||||
var control = createTemporaryObject(customButtonBox, testCase, {})
|
||||
verify(control)
|
||||
|
||||
var listView = control.contentItem
|
||||
waitForRendering(listView)
|
||||
|
||||
var button = control.okButton
|
||||
verify(button)
|
||||
button.text = "some longer text";
|
||||
|
||||
// The button should never go outside of the box.
|
||||
tryVerify(function() { return button.mapToItem(control, 0, 0).x >= 0 },
|
||||
1000, "Expected left edge of button to be within left edge of DialogButtonBox (i.e. greater than or equal to 0)" +
|
||||
", but it's " + button.mapToItem(control, 0, 0).x)
|
||||
tryVerify(function() { return button.mapToItem(control, 0, 0).x + button.width <= control.width },
|
||||
1000, "Expected right edge of button to be within right edge of DialogButtonBox (i.e. less than or equal to " +
|
||||
control.width + "), but it's " + (button.mapToItem(control, 0, 0).x + button.width))
|
||||
}
|
||||
|
||||
Component {
|
||||
id: customButtonBoxTwoButtons
|
||||
|
||||
DialogButtonBox {
|
||||
objectName: "customButtonBoxTwoButtons"
|
||||
alignment: Qt.AlignRight
|
||||
|
||||
property alias okButton: okButton
|
||||
|
||||
Button {
|
||||
id: okButton
|
||||
text: "OK"
|
||||
|
||||
DialogButtonBox.buttonRole: DialogButtonBox.AcceptRole
|
||||
}
|
||||
Button {
|
||||
text: "Cancel"
|
||||
|
||||
DialogButtonBox.buttonRole: DialogButtonBox.RejectRole
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// QTBUG-72886
|
||||
function test_twoCustomButtonsChangeText() {
|
||||
var control = createTemporaryObject(customButtonBoxTwoButtons, testCase, {})
|
||||
verify(control)
|
||||
|
||||
var listView = control.contentItem
|
||||
waitForRendering(listView)
|
||||
|
||||
var button = control.okButton
|
||||
button.text = "some longer text";
|
||||
// The button should never go outside of the box.
|
||||
tryVerify(function() { return button.mapToItem(control, 0, 0).x >= 0 },
|
||||
1000, "Expected left edge of button to be within left edge of DialogButtonBox (i.e. greater than or equal to 0)" +
|
||||
", but it's " + button.mapToItem(control, 0, 0).x)
|
||||
tryVerify(function() { return button.mapToItem(control, 0, 0).x + button.width <= control.width },
|
||||
1000, "Expected right edge of button to be within right edge of DialogButtonBox (i.e. less than or equal to " +
|
||||
control.width + "), but it's " + (button.mapToItem(control, 0, 0).x + button.width))
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue