qquickpopuppositioner: fix popup mirroring

Fixed a bug where QQuickPopupPositioner::reposition() was confusing
coordinates obtained by mapToSource() and mapToItem() during mirroring
of the popup. This leads to popups at the wrong places, e.g. when a
rotated combobox has not enough space to unroll its popup.

Translate the coordinates using mapToItem instead of mapToSource after
computing the alternative position of the popup (e.g. when it can not
be unrolled correctly).

Add a test to check if the combobox popup appears at the right places.
Skip native styles as they might be pushing the popup around, same
for Android as they might have too small screens (which involves
more positioning logic then just the mirroring).

Fixes: QTBUG-105148
Pick-to: 5.15 6.4 6.2 6.5
Change-Id: I0033509a8824e3a71698f91ef832b94527d8e2c8
Reviewed-by: Oliver Eftevaag <oliver.eftevaag@qt.io>
This commit is contained in:
Sami Shalayel 2022-12-14 14:15:50 +01:00
parent e09b72fe10
commit 5c1d96b70a
4 changed files with 211 additions and 2 deletions

View File

@ -129,14 +129,18 @@ void QQuickPopupPositioner::reposition()
// if the popup doesn't fit horizontally inside the window, try flipping it around (left <-> right)
if (p->allowHorizontalFlip && (rect.left() < bounds.left() || rect.right() > bounds.right())) {
const QRectF flipped(m_parentItem->mapToScene(QPointF(m_parentItem->width() - p->x - rect.width(), p->y)), rect.size());
const QPointF newTopLeft(m_parentItem->width() - p->x - rect.width(), p->y);
const QRectF flipped(m_parentItem->mapToItem(popupItem->parentItem(), newTopLeft),
rect.size());
if (flipped.intersected(bounds).width() > rect.intersected(bounds).width())
rect.moveLeft(flipped.left());
}
// if the popup doesn't fit vertically inside the window, try flipping it around (above <-> below)
if (p->allowVerticalFlip && (rect.top() < bounds.top() || rect.bottom() > bounds.bottom())) {
const QRectF flipped(m_parentItem->mapToScene(QPointF(p->x, m_parentItem->height() - p->y - rect.height())), rect.size());
const QPointF newTopLeft(p->x, m_parentItem->height() - p->y - rect.height());
const QRectF flipped(m_parentItem->mapToItem(popupItem->parentItem(), newTopLeft),
rect.size());
if (flipped.intersected(bounds).height() > rect.intersected(bounds).height())
rect.moveTop(flipped.top());
}

View File

@ -0,0 +1,26 @@
// Copyright (C) 2022 The Qt Company Ltd.
// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR BSD-3-Clause
import QtQuick
import QtQuick.Controls
Window {
width: 400
height: 400
contentItem.rotation: 180
ComboBox {
objectName: "first"
x: 100
y: 300 // is missing space, needs to unroll in the "mirrored" direction
model: ["First", "Second", "Third", "Fourth", "Fifth"]
}
ComboBox {
objectName: "second"
x: 200
y: 100 // has enough space to unroll
model: ["A", "B", "C"]
}
}

View File

@ -0,0 +1,26 @@
// Copyright (C) 2022 The Qt Company Ltd.
// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR BSD-3-Clause
import QtQuick
import QtQuick.Controls
Window {
width: 400
height: 400
contentItem.rotation: 90
ComboBox {
objectName: "first"
x: 100
y: 320 // is missing space, needs to unroll in the "mirrored" direction
model: ["First", "Second", "Third", "Fourth", "Fifth"]
}
ComboBox {
objectName: "second"
x: 200
y: 100 // has enough space to unroll
model: ["A", "B", "C"]
}
}

View File

@ -89,12 +89,16 @@ private slots:
void shrinkPopupThatWasLargerThanWindow_data();
void shrinkPopupThatWasLargerThanWindow();
void relativeZOrder();
void mirroredCombobox();
void rotatedCombobox();
private:
static bool hasWindowActivation();
QScopedPointer<QPointingDevice> touchScreen = QScopedPointer<QPointingDevice>(QTest::createTouchDevice());
};
using namespace Qt::StringLiterals;
tst_QQuickPopup::tst_QQuickPopup()
: QQmlDataTest(QT_QMLTEST_DATADIR)
{
@ -1958,6 +1962,155 @@ void tst_QQuickPopup::relativeZOrder()
QCOMPARE(overlayPrivate->paintOrderChildItems().last(), subDialog->popupItem());
}
void tst_QQuickPopup::mirroredCombobox()
{
#ifdef Q_OS_ANDROID
// Android screens might be pretty small, such that additional
// repositioning (apart from the mirroring) will happen to the
// popups and mess up the expected positions below.
QSKIP("Skipping test for Android.");
#endif
QStringList nativeStyles;
nativeStyles.append(u"macOS"_s);
nativeStyles.append(u"iOS"_s);
nativeStyles.append(u"Windows"_s);
if (nativeStyles.contains(QQuickStyle::name()))
QSKIP("Skipping test for native styles: they might rearrange their combobox the way they "
"want.");
QQuickControlsApplicationHelper helper(this, "mirroredCombobox.qml");
QVERIFY2(helper.ready, helper.failureMessage());
QQuickWindow *window = helper.window;
window->show();
QVERIFY(QTest::qWaitForWindowExposed(window));
{
QQuickComboBox *comboBox = window->findChild<QQuickComboBox *>("first");
QVERIFY(comboBox);
QQuickPopup *popup = comboBox->popup();
QVERIFY(popup);
popup->open();
QTRY_COMPARE(popup->isVisible(), true);
const QPointF popupPos(popup->contentItem()->mapToItem(comboBox->parentItem(),
popup->contentItem()->position()));
const QSizeF popupSize(popup->contentItem()->size());
// ignore popup.{top,bottom}Padding() as not included in popup->contentItem()->size()
// some styles prefer to draw the popup "over" (in z-axis direction) the combobox to hide
// the combobox
const bool styleDrawsPopupOverCombobox =
comboBox->position().y() - popupSize.height() + comboBox->size().height()
== popupPos.y();
// some styles prefer to draw the popup below (in y-axis direction) the combobox
const bool styleDrawsPopupBelowCombobox =
comboBox->position().y() - popupSize.height() + comboBox->topPadding()
== popupPos.y();
QVERIFY(styleDrawsPopupOverCombobox || styleDrawsPopupBelowCombobox);
popup->close();
}
{
QQuickComboBox *comboBox = window->findChild<QQuickComboBox *>("second");
QVERIFY(comboBox);
QQuickPopup *popup = comboBox->popup();
QVERIFY(popup);
popup->open();
QTRY_COMPARE(popup->isVisible(), true);
const QPointF popupPos(popup->contentItem()->mapToItem(comboBox->parentItem(),
popup->contentItem()->position()));
// some styles prefer to draw the popup "over" (in z-axis direction) the combobox to hide
// the combobox
const bool styleDrawsPopupOverCombobox = comboBox->position().y() + comboBox->topPadding()
+ popup->topPadding() + popup->bottomPadding()
== popupPos.y();
// some styles prefer to draw the popup above (in y-axis direction) the combobox
const bool styleDrawsPopupAboveCombobox =
comboBox->position().y() + comboBox->height() - comboBox->topPadding()
== popupPos.y();
QVERIFY(styleDrawsPopupOverCombobox || styleDrawsPopupAboveCombobox);
popup->close();
}
}
void tst_QQuickPopup::rotatedCombobox()
{
#ifdef Q_OS_ANDROID
// Android screens might be pretty small, such that additional
// repositioning (apart from the rotating) will happen to the
// popups and mess up the expected positions below.
QSKIP("Skipping test for Android.");
#endif
QStringList nativeStyles;
nativeStyles.append(u"macOS"_s);
nativeStyles.append(u"iOS"_s);
nativeStyles.append(u"Windows"_s);
if (nativeStyles.contains(QQuickStyle::name()))
QSKIP("Skipping test for native styles: they might rearrange their combobox the way they "
"want.");
QQuickControlsApplicationHelper helper(this, "rotatedCombobox.qml");
QVERIFY2(helper.ready, helper.failureMessage());
QQuickWindow *window = helper.window;
window->show();
QVERIFY(QTest::qWaitForWindowExposed(window));
{
QQuickComboBox *comboBox = window->findChild<QQuickComboBox *>("first");
QVERIFY(comboBox);
QQuickPopup *popup = comboBox->popup();
QVERIFY(popup);
popup->open();
QTRY_COMPARE(popup->isVisible(), true);
const QPointF popupPos(popup->contentItem()->mapToItem(comboBox->parentItem(),
popup->contentItem()->position()));
const QSizeF popupSize(popup->contentItem()->size());
// ignore popup.{left,right}Padding() as not included in popup->contentItem()->size()
// some styles prefer to draw the popup "over" (in z-axis direction) the combobox to hide
// the combobox
const bool styleDrawsPopupOverCombobox =
comboBox->position().x() - popupSize.width() + comboBox->width() == popupPos.x();
// some styles prefer to draw the popup right (in x-axis direction) of the combobox
const bool styleDrawsPopupBelowCombobox =
comboBox->position().x() - popupSize.width() - comboBox->leftPadding()
== popupPos.x();
QVERIFY(styleDrawsPopupOverCombobox || styleDrawsPopupBelowCombobox);
}
{
QQuickComboBox *comboBox = window->findChild<QQuickComboBox *>("second");
QVERIFY(comboBox);
QQuickPopup *popup = comboBox->popup();
QVERIFY(popup);
popup->open();
QTRY_COMPARE(popup->isVisible(), true);
const QPointF popupPos(popup->contentItem()->mapToItem(comboBox->parentItem(),
popup->contentItem()->position()));
// some styles prefer to draw the popup "over" (in z-axis direction) the combobox to hide
// the combobox
const bool styleDrawsPopupOverCombobox = comboBox->position().x() + comboBox->leftPadding()
+ popup->leftPadding() + popup->rightPadding()
== popupPos.x();
// some styles prefer to draw the popup left (in y-axis direction) of the combobox
const bool styleDrawsPopupAboveCombobox =
comboBox->position().x() + comboBox->width() - comboBox->leftPadding()
== popupPos.x();
QVERIFY(styleDrawsPopupOverCombobox || styleDrawsPopupAboveCombobox);
popup->close();
}
}
QTEST_QUICKCONTROLS_MAIN(tst_QQuickPopup)
#include "tst_qquickpopup.moc"