From 0525d640cd11ddced2ec418be182c585204fc45f Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Fri, 12 Apr 2019 15:21:42 +0200 Subject: Fix MenuItem width not matching Menu's available width Short version: There are currently two problems with MenuItems: - Mirrored MenuItems don't fill the Menu's available width. - MenuItem does not fill the Menu's available width when changed after Component completion. This patch fixes both of them by listening to geometry changes in both the contentItem and individual menu items, and setting the explicit width of those menu items when either changes. Longer version: The first problem can be seen whenever the MenuItem's implicitWidth changes: - QQmlEngine::retranslate() is called, causing all bindings to be re-evaluated - The MenuItem's font size changes - The MenuItem's icon size changes - etc. We fix this by making Menu listen to the width of each of its MenuItems and call resizeItem() if it doesn't have an explicit width. The second problem can be seen when e.g. resizing a Menu to account for new items that are wider and hence require more space. This can be fixed by listening to width changes in Menu's contentItem, which was actually done in earlier versions but (probably accidentally) removed in 482ecb0f. I had tried to solve both issues by setting the explicit width of MenuItem to the width of its Menu, or undefined if it has none (which means it reverts to its implicit width). However, this does not account for e.g. MenuSeparator and custom items that can be added to Menu - they should also have their width fill the Menu automatically if they don't have an explicit width set. Change-Id: I95dd0da0919a1e297f2e2030da746ff1f1a17644 Fixes: QTBUG-75051 Fixes: QTBUG-75142 Reviewed-by: Richard Moe Gustavsen --- tests/auto/qquickmenu/tst_qquickmenu.cpp | 150 +++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) (limited to 'tests/auto/qquickmenu/tst_qquickmenu.cpp') diff --git a/tests/auto/qquickmenu/tst_qquickmenu.cpp b/tests/auto/qquickmenu/tst_qquickmenu.cpp index 1b00817f..49fdc066 100644 --- a/tests/auto/qquickmenu/tst_qquickmenu.cpp +++ b/tests/auto/qquickmenu/tst_qquickmenu.cpp @@ -94,6 +94,13 @@ private slots: void scrollable(); void disableWhenTriggered_data(); void disableWhenTriggered(); + void menuItemWidth_data(); + void menuItemWidth(); + void menuItemWidthAfterMenuWidthChanged_data(); + void menuItemWidthAfterMenuWidthChanged(); + void menuItemWidthAfterImplicitWidthChanged_data(); + void menuItemWidthAfterImplicitWidthChanged(); + void menuItemWidthAfterRetranslate(); }; void tst_QQuickMenu::defaults() @@ -1668,6 +1675,149 @@ void tst_QQuickMenu::disableWhenTriggered() } } +void tst_QQuickMenu::menuItemWidth_data() +{ + QTest::addColumn("mirrored"); + + QTest::newRow("non-mirrored") << false; + QTest::newRow("mirrored") << true; +} + +void tst_QQuickMenu::menuItemWidth() +{ + QFETCH(bool, mirrored); + + QQuickApplicationHelper helper(this, QLatin1String("menuItemWidths.qml")); + QQuickApplicationWindow *window = helper.appWindow; + window->show(); + QVERIFY(QTest::qWaitForWindowActive(window)); + + if (mirrored) + window->setLocale(QLocale("ar_EG")); + + QQuickMenu *menu = window->property("menu").value(); + QVERIFY(menu); + menu->open(); + QTRY_VERIFY(menu->isOpened()); + for (int i = 0; i < menu->count(); ++i) + QCOMPARE(menu->itemAt(i)->width(), menu->availableWidth()); +} + +void tst_QQuickMenu::menuItemWidthAfterMenuWidthChanged_data() +{ + QTest::addColumn("mirrored"); + + QTest::newRow("non-mirrored") << false; + QTest::newRow("mirrored") << true; +} + +void tst_QQuickMenu::menuItemWidthAfterMenuWidthChanged() +{ + QFETCH(bool, mirrored); + + QQuickApplicationHelper helper(this, QLatin1String("menuItemWidths.qml")); + QQuickApplicationWindow *window = helper.appWindow; + window->show(); + QVERIFY(QTest::qWaitForWindowActive(window)); + + if (mirrored) + window->setLocale(QLocale("ar_EG")); + + QQuickMenu *menu = window->property("menu").value(); + QVERIFY(menu); + menu->open(); + QTRY_VERIFY(menu->isOpened()); + for (int i = 0; i < menu->count(); ++i) { + // Check that the width of menu items is correct before we resize the menu. + const QQuickItem *item = menu->itemAt(i); + QVERIFY2(qFuzzyCompare(item->width(), menu->availableWidth()), + qPrintable(QString::fromLatin1("Expected width of %1 to be %2, but it's %3") + .arg(item->objectName()).arg(menu->availableWidth()).arg(item->width()))); + } + + menu->setWidth(menu->width() + 10); + + // Check that the width of menu items is correct after we resize the menu. + for (int i = 0; i < menu->count(); ++i) { + // Check that the width of menu items is correct after we resize the menu. + const QQuickItem *item = menu->itemAt(i); + QVERIFY2(qFuzzyCompare(item->width(), menu->availableWidth()), + qPrintable(QString::fromLatin1("Expected width of %1 to be %2, but it's %3") + .arg(item->objectName()).arg(menu->availableWidth()).arg(item->width()))); + } +} + +void tst_QQuickMenu::menuItemWidthAfterImplicitWidthChanged_data() +{ + QTest::addColumn("mirrored"); + + QTest::newRow("non-mirrored") << false; + QTest::newRow("mirrored") << true; +} + +void tst_QQuickMenu::menuItemWidthAfterImplicitWidthChanged() +{ + QFETCH(bool, mirrored); + + QQuickApplicationHelper helper(this, QLatin1String("menuItemWidths.qml")); + QQuickApplicationWindow *window = helper.appWindow; + window->show(); + QVERIFY(QTest::qWaitForWindowActive(window)); + + if (mirrored) + window->setLocale(QLocale("ar_EG")); + + QQuickMenu *menu = window->property("menu").value(); + QVERIFY(menu); + menu->open(); + QTRY_VERIFY(menu->isOpened()); + // Check that the width of the menu item is correct before we change its font size. + QQuickMenuItem *menuItem = qobject_cast(menu->itemAt(0)); + QCOMPARE(menuItem->width(), menu->availableWidth()); + + // Add some text to increase the implicitWidth of the MenuItem. + const qreal oldImplicitWidth = menuItem->implicitWidth(); + for (int i = 0; menuItem->implicitWidth() <= oldImplicitWidth; ++i) { + menuItem->setText(menuItem->text() + QLatin1String("---")); + if (i == 100) + QFAIL("Shouldn't need 100 iterations to increase MenuItem's implicitWidth; something is wrong here"); + } + + // Check that the width of the menu item is correct after we change its font size. + QCOMPARE(menuItem->width(), menu->availableWidth()); +} + +void tst_QQuickMenu::menuItemWidthAfterRetranslate() +{ + QQuickApplicationHelper helper(this, QLatin1String("menuItemWidths.qml")); + QQuickApplicationWindow *window = helper.appWindow; + window->show(); + QVERIFY(QTest::qWaitForWindowActive(window)); + + QQuickMenu *menu = window->property("menu").value(); + QVERIFY(menu); + menu->open(); + QTRY_VERIFY(menu->isOpened()); + for (int i = 0; i < menu->count(); ++i) { + // Check that the width of each menu item is correct before we retranslate. + const QQuickItem *item = menu->itemAt(i); + QVERIFY2(qFuzzyCompare(item->width(), menu->availableWidth()), + qPrintable(QString::fromLatin1("Expected width of %1 to be %2, but it's %3") + .arg(item->objectName()).arg(menu->availableWidth()).arg(item->width()))); + } + + // Call retranslate() and cause all bindings to be re-evaluated. + helper.engine.retranslate(); + + for (int i = 0; i < menu->count(); ++i) { + // Check that the width of each menu item is correct after we retranslate. + const QQuickItem *item = menu->itemAt(i); + QVERIFY2(qFuzzyCompare(item->width(), menu->availableWidth()), + qPrintable(QString::fromLatin1("Expected width of %1 to be %2, but it's %3") + .arg(item->objectName()).arg(menu->availableWidth()).arg(item->width()))); + } +} + QTEST_QUICKCONTROLS_MAIN(tst_QQuickMenu) #include "tst_qquickmenu.moc" -- cgit v1.2.3