From b4c54935fb2896444f244bfee3c53f8ff18306da Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Tue, 22 Oct 2019 17:00:49 +0200 Subject: SplitView: fix issue where Repeater items were not created SplitView's contentItem is lazily created whenever contentItem() is called. When adding regular, standalone items, they will go through QQuickContainer::addItem(), which eventually calls contentItem(). This case works fine. Repeaters, on the other hand, call setTransparentForPositioner(true), which QQuickContainerPrivate::contentData_append() checks for, and instead of calling addItem(), reparents the Repeater to effectiveContentItem() with this line: item->setParentItem(effectiveContentItem(p->contentItem)); If this happens before the contentItem is created, then the Repeater has no parentItem and won't generate any items. So, instead of using the contentItem member directly, call contentItem() to create it if it doesn't exist. Fixes: QTBUG-79302 Change-Id: I258f7420d2fea843ed045d569f80e92fe1f507d2 Reviewed-by: Mitch Curtis --- tests/auto/controls/data/tst_splitview.qml | 69 ++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) (limited to 'tests') diff --git a/tests/auto/controls/data/tst_splitview.qml b/tests/auto/controls/data/tst_splitview.qml index 76572a00..c125b99e 100644 --- a/tests/auto/controls/data/tst_splitview.qml +++ b/tests/auto/controls/data/tst_splitview.qml @@ -146,6 +146,13 @@ TestCase { implicitWidth: defaultHorizontalHandleWidth implicitHeight: defaultVerticalHandleHeight color: "#444" + + Text { + text: parent.x + "," + parent.y + " " + parent.width + "x" + parent.height + color: "white" + anchors.centerIn: parent + rotation: 90 + } } } @@ -834,6 +841,36 @@ TestCase { } } + Component { + id: repeaterSplitViewComponent + + SplitView { + anchors.fill: parent + handle: handleComponent + + property alias repeater: repeater + + Repeater { + id: repeater + model: 3 + delegate: Rectangle { + objectName: "rectDelegate" + index + + SplitView.preferredWidth: 25 + + color: "#aaff0000" + + Text { + text: parent.x + "," + parent.y + " " + parent.width + "x" + parent.height + color: "white" + rotation: 90 + anchors.centerIn: parent + } + } + } + } + } + function test_dragHandle_data() { var splitViewWidth = testCase.width - splitViewMargins * 2 var splitViewHeight = testCase.height - splitViewMargins * 2 @@ -1092,6 +1129,28 @@ TestCase { { x: 25 + 100 + defaultHorizontalHandleWidth, y: 0, width: defaultHorizontalHandleWidth, height: splitViewHeight }, { x: 25 + 100 + defaultHorizontalHandleWidth * 2, y: 0, width: splitViewWidth - 100, height: splitViewHeight } ] + }, + { + tag: "repeater", + component: repeaterSplitViewComponent, + orientation: Qt.Horizontal, + fillIndex: 2, + handleIndex: 1, + newHandlePos: Qt.point(200, testCase.height / 2), + expectedGeometriesBeforeDrag: [ + { x: 0, y: 0, width: 25, height: splitViewHeight }, + { x: 25, y: 0, width: defaultHorizontalHandleWidth, height: splitViewHeight }, + { x: 25 + defaultHorizontalHandleWidth, y: 0, width: 25, height: splitViewHeight }, + { x: 25 * 2 + defaultHorizontalHandleWidth, y: 0, width: defaultHorizontalHandleWidth, height: splitViewHeight }, + { x: 25 * 2 + defaultHorizontalHandleWidth * 2, y: 0, width: splitViewWidth - 70 , height: splitViewHeight } + ], + expectedGeometriesAfterDrag: [ + { x: 0, y: 0, width: 25, height: splitViewHeight }, + { x: 25, y: 0, width: defaultHorizontalHandleWidth, height: splitViewHeight }, + { x: 25 + defaultHorizontalHandleWidth, y: 0, width: 105, height: splitViewHeight }, + { x: 140, y: 0, width: defaultHorizontalHandleWidth, height: splitViewHeight }, + { x: 150, y: 0, width: 150, height: splitViewHeight } + ] } ] return data @@ -1122,6 +1181,7 @@ TestCase { var targetHandle = handles[data.handleIndex] mousePress(targetHandle) verify(control.resizing) + // newHandlePos is in scene coordinates, so map it to coordinates local to the handle. var localPos = testCase.mapToItem(targetHandle, data.newHandlePos.x, data.newHandlePos.y) mouseMove(targetHandle, localPos.x - targetHandle.width / 2, localPos.y - targetHandle.height / 2) verify(control.resizing) @@ -1957,4 +2017,13 @@ TestCase { // Shouldn't be an assertion failure. control.visible = false } + + // QTBUG-79302: ensure that the Repeater's items are actually generated. + // test_dragHandle:repeater tests dragging behavior with a Repeater. + function test_repeater(data) { + var control = createTemporaryObject(repeaterSplitViewComponent, testCase) + verify(control) + compare(control.repeater.count, 3) + compare(control.contentChildren.length, 3) + } } -- cgit v1.2.3 From 21e3c3eff23c34f9e4b85794e336380d9f6815f6 Mon Sep 17 00:00:00 2001 From: Wang Chuan Date: Mon, 28 Oct 2019 16:53:02 +0800 Subject: QQuickPopup: try to grab shortcut when component completed If closePolicy of Popup is set to CloseOnEscape and the Popup is completed, shortcut will register to QGuiApplication to let Popup respond to Escape key. However if Popup is set to visible in creation, even if we set closePolicy to CloseOnEscape, the shortcut won't be registered. [ChangeLog][Controls][QQuickPopup] Fixed the issue that Popup doesn't respond to CloseOnEscape if the initial value of visible is true Fixes: QTBUG-79326 Change-Id: I90c6805e2b4d567a6e0d33d43a75fedcfc5416b3 Reviewed-by: Mitch Curtis --- .../qquickpopup/data/closeOnEscapeWithVisiblePopup.qml | 16 ++++++++++++++++ tests/auto/qquickpopup/tst_qquickpopup.cpp | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 tests/auto/qquickpopup/data/closeOnEscapeWithVisiblePopup.qml (limited to 'tests') diff --git a/tests/auto/qquickpopup/data/closeOnEscapeWithVisiblePopup.qml b/tests/auto/qquickpopup/data/closeOnEscapeWithVisiblePopup.qml new file mode 100644 index 00000000..b9606eb2 --- /dev/null +++ b/tests/auto/qquickpopup/data/closeOnEscapeWithVisiblePopup.qml @@ -0,0 +1,16 @@ +import QtQuick 2.13 +import QtQuick.Window 2.13 +import QtQuick.Controls 2.13 + +Window { + width: 400 + height: 400 + Popup { + objectName: "popup" + visible: true + width: 200 + height: 200 + anchors.centerIn: parent + closePolicy: Popup.CloseOnEscape + } +} diff --git a/tests/auto/qquickpopup/tst_qquickpopup.cpp b/tests/auto/qquickpopup/tst_qquickpopup.cpp index c36edd6d..7da20c37 100644 --- a/tests/auto/qquickpopup/tst_qquickpopup.cpp +++ b/tests/auto/qquickpopup/tst_qquickpopup.cpp @@ -82,6 +82,7 @@ private slots: void cursorShape(); void componentComplete(); void closeOnEscapeWithNestedPopups(); + void closeOnEscapeWithVisiblePopup(); void enabled(); void orientation_data(); void orientation(); @@ -1020,6 +1021,21 @@ void tst_QQuickPopup::closeOnEscapeWithNestedPopups() QCOMPARE(stackView->depth(), 1); } +void tst_QQuickPopup::closeOnEscapeWithVisiblePopup() +{ + QQuickApplicationHelper helper(this, QStringLiteral("closeOnEscapeWithVisiblePopup.qml")); + QQuickWindow *window = helper.window; + window->show(); + QVERIFY(QTest::qWaitForWindowExposed(window)); + + QQuickPopup *popup = window->findChild("popup"); + QVERIFY(popup); + QTRY_VERIFY(popup->isOpened()); + + QTest::keyClick(window, Qt::Key_Escape); + QTRY_VERIFY(!popup->isVisible()); +} + void tst_QQuickPopup::enabled() { QQuickPopup popup; -- cgit v1.2.3