From 49ffc6e6af83b295c67fd119b79c925879cc292e Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Thu, 27 Feb 2020 15:36:20 +0100 Subject: ComboBox: add implicitContentWidthPolicy [ChangeLog][Controls][ComboBox] Added implicitContentWidthPolicy, which controls how the implicitContentWidth of the ComboBox is calculated. This can be used to automatically ensure that text is not elided. Fixes: QTBUG-78281 Change-Id: If669fa4ef05e5da498992843469000ef39c335ec Reviewed-by: Shawn Rutledge --- src/quicktemplates2/qquickcombobox.cpp | 178 +++++++++++++++++++++++++++- src/quicktemplates2/qquickcombobox_p.h | 17 +++ tests/auto/controls/data/tst_combobox.qml | 186 +++++++++++++++++++++++++++++- 3 files changed, 374 insertions(+), 7 deletions(-) diff --git a/src/quicktemplates2/qquickcombobox.cpp b/src/quicktemplates2/qquickcombobox.cpp index 85f135c7..426adbd3 100644 --- a/src/quicktemplates2/qquickcombobox.cpp +++ b/src/quicktemplates2/qquickcombobox.cpp @@ -54,10 +54,13 @@ #include #include #include +#include #include QT_BEGIN_NAMESPACE +Q_LOGGING_CATEGORY(lcCalculateWidestTextWidth, "qt.quick.controls.combobox.calculatewidesttextwidth") + /*! \qmltype ComboBox \inherits Control @@ -230,6 +233,7 @@ public: void modelUpdated(); void countChanged(); + QString effectiveTextRole() const; void updateEditText(); void updateCurrentText(); void updateCurrentValue(); @@ -265,6 +269,10 @@ public: void itemImplicitWidthChanged(QQuickItem *item) override; void itemImplicitHeightChanged(QQuickItem *item) override; + virtual qreal getContentWidth() const override; + qreal calculateWidestTextWidth() const; + void maybeUpdateImplicitContentWidth(); + static void hideOldPopup(QQuickPopup *popup); QPalette defaultPalette() const override { return QQuickTheme::palette(QQuickTheme::ComboBox); } @@ -277,8 +285,10 @@ public: bool keyNavigating = false; bool hasDisplayText = false; bool hasCurrentIndex = false; + bool hasCalculatedWidestText = false; int highlightedIndex = -1; int currentIndex = -1; + QQuickComboBox::ImplicitContentWidthPolicy implicitContentWidthPolicy = QQuickComboBox::ContentItemImplicitWidth; QVariant model; QString textRole; QString currentText; @@ -411,8 +421,12 @@ void QQuickComboBoxPrivate::createdItem(int index, QObject *object) void QQuickComboBoxPrivate::modelUpdated() { - if (!extra.isAllocated() || !extra->accepting) + if (componentComplete && (!extra.isAllocated() || !extra->accepting)) { updateCurrentTextAndValue(); + + if (implicitContentWidthPolicy == QQuickComboBox::WidestText) + updateImplicitContentSize(); + } } void QQuickComboBoxPrivate::countChanged() @@ -423,6 +437,11 @@ void QQuickComboBoxPrivate::countChanged() emit q->countChanged(); } +QString QQuickComboBoxPrivate::effectiveTextRole() const +{ + return textRole.isEmpty() ? QStringLiteral("modelData") : textRole; +} + void QQuickComboBoxPrivate::updateEditText() { Q_Q(QQuickComboBox); @@ -780,6 +799,72 @@ void QQuickComboBoxPrivate::itemImplicitHeightChanged(QQuickItem *item) emit q->implicitIndicatorHeightChanged(); } +qreal QQuickComboBoxPrivate::getContentWidth() const +{ + if (componentComplete) { + switch (implicitContentWidthPolicy) { + case QQuickComboBox::WidestText: + return calculateWidestTextWidth(); + case QQuickComboBox::WidestTextWhenCompleted: + if (!hasCalculatedWidestText) + return calculateWidestTextWidth(); + break; + default: + break; + } + } + + return QQuickControlPrivate::getContentWidth(); +} + +qreal QQuickComboBoxPrivate::calculateWidestTextWidth() const +{ + Q_Q(const QQuickComboBox); + if (!componentComplete) + return 0; + + const int count = q->count(); + if (count == 0) + return 0; + + auto textInput = qobject_cast(contentItem); + if (!textInput) + return 0; + + qCDebug(lcCalculateWidestTextWidth) << "calculating widest text from" << count << "items..."; + + // Avoid the index check and repeated calls to effectiveTextRole() + // that would result from calling textAt() in a loop. + const QString textRole = effectiveTextRole(); + auto textInputPrivate = QQuickTextInputPrivate::get(textInput); + qreal widest = 0; + for (int i = 0; i < count; ++i) { + const QString text = delegateModel->stringValue(i, textRole); + const qreal textImplicitWidth = textInputPrivate->calculateImplicitWidthForText(text); + widest = qMax(widest, textImplicitWidth); + } + + qCDebug(lcCalculateWidestTextWidth) << "... widest text is" << widest; + return widest; +} + +/*! + If the user requested it (and we haven't already done it, depending on the policy), + update the implicit content width to the largest text in the model. +*/ +void QQuickComboBoxPrivate::maybeUpdateImplicitContentWidth() +{ + if (!componentComplete) + return; + + if (implicitContentWidthPolicy == QQuickComboBox::ContentItemImplicitWidth + || (implicitContentWidthPolicy == QQuickComboBox::WidestTextWhenCompleted && hasCalculatedWidestText)) + return; + + updateImplicitContentWidth(); + hasCalculatedWidestText = true; +} + void QQuickComboBoxPrivate::hideOldPopup(QQuickPopup *popup) { if (!popup) @@ -885,6 +970,8 @@ void QQuickComboBox::setModel(const QVariant& m) d->updateCurrentTextAndValue(); } emit modelChanged(); + + d->maybeUpdateImplicitContentWidth(); } /*! @@ -1597,6 +1684,80 @@ void QQuickComboBox::setSelectTextByMouse(bool canSelect) d->extra.value().selectTextByMouse = canSelect; emit selectTextByMouseChanged(); } + +/*! + \since QtQuick.Controls 6.0 (Qt 6.0) + \qmlproperty enumeration QtQuick.Controls::ComboBox::implicitContentWidthPolicy + + This property controls how the \l implicitContentWidth of the ComboBox is + calculated. + + When the width of a ComboBox is not large enough to display text, that text + is elided. Depending on which parts of the text are elided, this can make + selecting an item difficult for the end user. An efficient way of ensuring + that a ComboBox is wide enough to avoid text being elided is to set a width + that is known to be large enough: + + \code + width: 300 + implicitContentWidthPolicy: ComboBox.ContentItemImplicitWidth + \endcode + + However, it is often not possible to know whether or not a hard-coded value + will be large enough, as the size of text depends on many factors, such as + font family, font size, translations, and so on. + + implicitContentWidthPolicy provides an easy way to control how the + implicitContentWidth is calculated, which in turn affects the + \l implicitWidth of the ComboBox and ensures that text will not be elided. + + The available values are: + + \value ContentItemImplicitWidth + The implicitContentWidth will default to that of the \l contentItem. + + This is the most efficient option, as no extra text layout is done. + \value WidestText + The implicitContentWidth will be set to the implicit width of the + the largest text for the given \l textRole every time the model + changes. + + This option should be used with smaller models, as it can be expensive. + \value WidestTextWhenCompleted + The implicitContentWidth will be set to the implicit width of the + the largest text for the given \l textRole once after + \l {QQmlParserStatus::componentComplete()}{component completion}. + + This option should be used with smaller models, as it can be expensive. + + The default value is \c ContentItemImplicitWidth. + + As this property only affects the \l implicitWidth of the ComboBox, setting + an explicit \l width can still result in eliding. + + \note This feature requires the contentItem to be a type derived from + \l TextInput. + + \note This feature requires text to be laid out, and can therefore be + expensive for large models or models whose contents are updated + frequently. +*/ +QQuickComboBox::ImplicitContentWidthPolicy QQuickComboBox::implicitContentWidthPolicy() const +{ + Q_D(const QQuickComboBox); + return d->implicitContentWidthPolicy; +} + +void QQuickComboBox::setImplicitContentWidthPolicy(QQuickComboBox::ImplicitContentWidthPolicy policy) +{ + Q_D(QQuickComboBox); + if (policy == d->implicitContentWidthPolicy) + return; + + d->implicitContentWidthPolicy = policy; + d->maybeUpdateImplicitContentWidth(); + emit implicitContentWidthPolicyChanged(); +} /*! \qmlmethod string QtQuick.Controls::ComboBox::textAt(int index) @@ -1611,8 +1772,7 @@ QString QQuickComboBox::textAt(int index) const if (!d->isValidIndex(index)) return QString(); - const QString effectiveTextRole = d->textRole.isEmpty() ? QStringLiteral("modelData") : d->textRole; - return d->delegateModel->stringValue(index, effectiveTextRole); + return d->delegateModel->stringValue(index, d->effectiveTextRole()); } /*! @@ -1894,6 +2054,11 @@ void QQuickComboBox::componentComplete() setCurrentIndex(0); else d->updateCurrentTextAndValue(); + + // If the widest text was already calculated in the call to + // QQmlDelegateModel::componentComplete() above, then we shouldn't do it here too. + if (!d->hasCalculatedWidestText) + d->maybeUpdateImplicitContentWidth(); } } @@ -1907,6 +2072,13 @@ void QQuickComboBox::itemChange(QQuickItem::ItemChange change, const QQuickItem: } } +void QQuickComboBox::fontChange(const QFont &newFont, const QFont &oldFont) +{ + Q_D(QQuickComboBox); + QQuickControl::fontChange(newFont, oldFont); + d->maybeUpdateImplicitContentWidth(); +} + void QQuickComboBox::contentItemChange(QQuickItem *newItem, QQuickItem *oldItem) { Q_D(QQuickComboBox); diff --git a/src/quicktemplates2/qquickcombobox_p.h b/src/quicktemplates2/qquickcombobox_p.h index 4874eb5d..18c1275c 100644 --- a/src/quicktemplates2/qquickcombobox_p.h +++ b/src/quicktemplates2/qquickcombobox_p.h @@ -94,6 +94,9 @@ class Q_QUICKTEMPLATES2_PRIVATE_EXPORT QQuickComboBox : public QQuickControl Q_PROPERTY(QString valueRole READ valueRole WRITE setValueRole NOTIFY valueRoleChanged FINAL REVISION 14) // 2.15 (Qt 5.15) Q_PROPERTY(bool selectTextByMouse READ selectTextByMouse WRITE setSelectTextByMouse NOTIFY selectTextByMouseChanged FINAL REVISION 15) + // TODO: 6.0 (Qt 6.0) - QTBUG-84190 + Q_PROPERTY(ImplicitContentWidthPolicy implicitContentWidthPolicy READ implicitContentWidthPolicy + WRITE setImplicitContentWidthPolicy NOTIFY implicitContentWidthPolicyChanged FINAL REVISION 15) public: explicit QQuickComboBox(QQuickItem *parent = nullptr); @@ -175,6 +178,17 @@ public: bool selectTextByMouse() const; void setSelectTextByMouse(bool canSelect); + // 6.0 (Qt 6.0) + enum ImplicitContentWidthPolicy { + ContentItemImplicitWidth, + WidestText, + WidestTextWhenCompleted + }; + Q_ENUM(ImplicitContentWidthPolicy) + + ImplicitContentWidthPolicy implicitContentWidthPolicy() const; + void setImplicitContentWidthPolicy(ImplicitContentWidthPolicy policy); + public Q_SLOTS: void incrementCurrentIndex(); void decrementCurrentIndex(); @@ -214,6 +228,8 @@ Q_SIGNALS: Q_REVISION(14) void currentValueChanged(); // 2.15 (Qt 5.15) Q_REVISION(15) void selectTextByMouseChanged(); + // 6.0 (Qt 6.0) + Q_REVISION(6, 0) void implicitContentWidthPolicyChanged(); protected: bool eventFilter(QObject *object, QEvent *event) override; @@ -231,6 +247,7 @@ protected: void componentComplete() override; void itemChange(ItemChange change, const ItemChangeData &value) override; + void fontChange(const QFont &newFont, const QFont &oldFont) override; void contentItemChange(QQuickItem *newItem, QQuickItem *oldItem) override; void localeChange(const QLocale &newLocale, const QLocale &oldLocale) override; diff --git a/tests/auto/controls/data/tst_combobox.qml b/tests/auto/controls/data/tst_combobox.qml index 64a525bf..7369f263 100644 --- a/tests/auto/controls/data/tst_combobox.qml +++ b/tests/auto/controls/data/tst_combobox.qml @@ -48,10 +48,10 @@ ** ****************************************************************************/ -import QtQuick 2.14 -import QtQuick.Window 2.2 -import QtTest 1.0 -import QtQuick.Controls 2.12 +import QtQuick 2.15 +import QtQuick.Window 2.15 +import QtTest 1.15 +import QtQuick.Controls 2.15 TestCase { id: testCase @@ -1970,4 +1970,182 @@ TestCase { compare(comboBox1.currentIndex, 9) compare(currentIndexSpy.count, 1) } + + Component { + id: appFontTextFieldComponent + TextField { + objectName: "appFontTextField" + font: Qt.application.font + // We don't want the background's implicit width to interfere with our tests, + // which are about implicit width of the contentItem of ComboBox, which is by default TextField. + background: null + } + } + + Component { + id: appFontContentItemComboBoxComponent + ComboBox { + // Override the contentItem so that the font doesn't vary between styles. + contentItem: TextField { + objectName: "appFontContentItemTextField" + // We do this just to be extra sure that the font never comes from the control, + // as we want it to match that of the TextField in the appFontTextFieldComponent. + font: Qt.application.font + background: null + } + } + } + + Component { + id: twoItemListModelComponent + + ListModel { + ListElement { display: "Short" } + ListElement { display: "Kinda long" } + } + } + + function appendedToModel(model, item) { + if (Array.isArray(model)) { + let newModel = model + newModel.push(item) + return newModel + } + + if (model.hasOwnProperty("append")) { + model.append({ display: item }) + // To account for the fact that changes to a JS array are not seen by the QML engine, + // we need to reassign the entire model and hence return it. For simplicity in the + // calling code, we do it for the ListModel code path too. It should be a no-op. + return model + } + + console.warn("appendedToModel: unrecognised model") + return undefined + } + + function removedFromModel(model, index, count) { + if (Array.isArray(model)) { + let newModel = model + newModel.splice(index, count) + return newModel + } + + if (model.hasOwnProperty("remove")) { + model.remove(index, count) + return model + } + + console.warn("removedFromModel: unrecognised model") + return undefined + } + + // We don't use a data-driven test for the policy because the checks vary a lot based on which enum we're testing. + function test_implicitContentWidthPolicy_ContentItemImplicitWidth() { + // Set ContentItemImplicitWidth and ensure that implicitContentWidth is as wide as the current item + // by comparing it against the implicitWidth of an identical TextField + let control = createTemporaryObject(appFontContentItemComboBoxComponent, testCase, { + model: ["Short", "Kinda long"], + implicitContentWidthPolicy: ComboBox.ContentItemImplicitWidth + }) + verify(control) + compare(control.implicitContentWidthPolicy, ComboBox.ContentItemImplicitWidth) + + let textField = createTemporaryObject(appFontTextFieldComponent, testCase) + verify(textField) + // Don't set any text on textField because we're not accounting for the widest + // text here, so we want to compare it against an empty TextField. + compare(control.implicitContentWidth, textField.implicitWidth) + + textField.font.pixelSize *= 2 + control.font.pixelSize *= 2 + compare(control.implicitContentWidth, textField.implicitWidth) + } + + function test_implicitContentWidthPolicy_WidestText_data() { + return [ + { tag: "Array", model: ["Short", "Kinda long"] }, + { tag: "ListModel", model: twoItemListModelComponent.createObject(testCase) }, + ] + } + + function test_implicitContentWidthPolicy_WidestText(data) { + let control = createTemporaryObject(appFontContentItemComboBoxComponent, testCase, { + model: data.model, + implicitContentWidthPolicy: ComboBox.WidestText + }) + verify(control) + compare(control.implicitContentWidthPolicy, ComboBox.WidestText) + + let textField = createTemporaryObject(appFontTextFieldComponent, testCase) + verify(textField) + textField.text = "Kinda long" + // Note that we don't need to change the current index here, as the implicitContentWidth + // is set to the implicitWidth of the TextField within the ComboBox as if it had the largest + // text from the model set on it. + // We use Math.ceil because TextInput uses qCeil internally, whereas the implicitWidth + // binding for TextField does not. + compare(Math.ceil(control.implicitContentWidth), Math.ceil(textField.implicitWidth)) + + // Add a longer item; it should affect the implicit content width. + let modifiedModel = appendedToModel(data.model, "Moderately long") + control.model = modifiedModel + textField.text = "Moderately long" + compare(Math.ceil(control.implicitContentWidth), Math.ceil(textField.implicitWidth)) + + // Remove the last two items; it should use the only remaining item's width. + modifiedModel = removedFromModel(data.model, 1, 2) + control.model = modifiedModel + compare(control.count, 1) + compare(control.currentText, "Short") + textField.text = "Short" + compare(Math.ceil(control.implicitContentWidth), Math.ceil(textField.implicitWidth)) + + // Changes in font should result in the implicitContentWidth being updated. + textField.font.pixelSize *= 2 + // We have to change the contentItem's font size manually since we break the + // style's binding to the control's font when we set Qt.application.font to it. + control.contentItem.font.pixelSize *= 2 + control.font.pixelSize *= 2 + compare(Math.ceil(control.implicitContentWidth), Math.ceil(textField.implicitWidth)) + } + + function test_implicitContentWidthPolicy_WidestTextWhenCompleted_data() { + return test_implicitContentWidthPolicy_WidestText_data() + } + + function test_implicitContentWidthPolicy_WidestTextWhenCompleted(data) { + let control = createTemporaryObject(appFontContentItemComboBoxComponent, testCase, { + model: data.model, + implicitContentWidthPolicy: ComboBox.WidestTextWhenCompleted + }) + verify(control) + compare(control.implicitContentWidthPolicy, ComboBox.WidestTextWhenCompleted) + + let textField = createTemporaryObject(appFontTextFieldComponent, testCase) + verify(textField) + textField.text = "Kinda long" + compare(Math.ceil(control.implicitContentWidth), Math.ceil(textField.implicitWidth)) + + // Add a longer item; it should not affect the implicit content width + // since we've already accounted for it once. + let modifiedModel = appendedToModel(data.model, "Moderately long") + control.model = modifiedModel + compare(Math.ceil(control.implicitContentWidth), Math.ceil(textField.implicitWidth)) + + // Remove the last two items; it should still not affect the implicit content width. + modifiedModel = removedFromModel(data.model, 1, 2) + control.model = modifiedModel + compare(control.count, 1) + compare(control.currentText, "Short") + compare(Math.ceil(control.implicitContentWidth), Math.ceil(textField.implicitWidth)) + + // Changes in font should not result in the implicitContentWidth being updated. + let oldTextFieldImplicitWidth = textField.implicitWidth + // Changes in font should result in the implicitContentWidth being updated. + textField.font.pixelSize *= 2 + control.contentItem.font.pixelSize *= 2 + control.font.pixelSize *= 2 + compare(Math.ceil(control.implicitContentWidth), Math.ceil(oldTextFieldImplicitWidth)) + } } -- cgit v1.2.3