From ce85cb16160394d616bc3dc4bf2a8915ec63b036 Mon Sep 17 00:00:00 2001 From: Matthias Rauter Date: Tue, 28 Mar 2023 14:04:47 +0200 Subject: Update displayText and value of SpinBox (live) The displayText of a SpinBox is now updated and sanitized (fixup) by the validator when the user edits the text of the contentItem. The value of a SpinBox can be updated along the displayText on demand (set property live to true) when the user edits the text. [ChangeLog][Controls] The value of a SpinBox can be updated automatically (live) when the user edits the text. The displayText of a SpinBox is updated and sanitized automatically. Fixes: QTBUG-64151 Fixes: QTBUG-85739 Fixes: QTBUG-103205 Change-Id: Id82856190e36d15be2dde8e38091893171f4dad2 Reviewed-by: Mitch Curtis --- src/quicktemplates/qquickspinbox.cpp | 152 ++++++-- src/quicktemplates/qquickspinbox_p.h | 6 + .../quickcontrols/controls/data/tst_spinbox.qml | 397 +++++++++++++++++++++ 3 files changed, 519 insertions(+), 36 deletions(-) diff --git a/src/quicktemplates/qquickspinbox.cpp b/src/quicktemplates/qquickspinbox.cpp index 5d98514746..b95b625721 100644 --- a/src/quicktemplates/qquickspinbox.cpp +++ b/src/quicktemplates/qquickspinbox.cpp @@ -90,8 +90,9 @@ public: int effectiveStepSize() const; - void updateDisplayText(bool modified = false); - void setDisplayText(const QString &displayText, bool modified = false); + void updateDisplayText(); + void setDisplayText(const QString &displayText); + void contentItemTextChanged(); bool upEnabled() const; void updateUpEnabled(); @@ -111,9 +112,13 @@ public: void itemImplicitWidthChanged(QQuickItem *item) override; void itemImplicitHeightChanged(QQuickItem *item) override; + QString evaluateTextFromValue(int val) const; + int evaluateValueFromText(const QString &text) const; + QPalette defaultPalette() const override { return QQuickTheme::palette(QQuickTheme::SpinBox); } bool editable = false; + bool live = false; bool wrap = false; int from = 0; int to = 99; @@ -150,23 +155,10 @@ int QQuickSpinBoxPrivate::boundValue(int value, bool wrap) const void QQuickSpinBoxPrivate::updateValue() { - Q_Q(QQuickSpinBox); if (contentItem) { QVariant text = contentItem->property("text"); if (text.isValid()) { - int val = 0; - QQmlEngine *engine = qmlEngine(q); - if (engine && valueFromText.isCallable()) { - QJSValue loc; -#if QT_CONFIG(qml_locale) - QV4::ExecutionEngine *v4 = QQmlEnginePrivate::getV4Engine(engine); - loc = QJSValuePrivate::fromReturnedValue(QQmlLocale::wrap(v4, locale)); -#endif - val = valueFromText.call(QJSValueList() << text.toString() << loc).toInt(); - } else { - val = locale.toInt(text.toString()); - } - setValue(val, /* allowWrap = */ false, /* modified = */ true); + setValue(evaluateValueFromText(text.toString()), /* allowWrap = */ false, /* modified = */ true); } } } @@ -188,7 +180,7 @@ bool QQuickSpinBoxPrivate::setValue(int newValue, bool allowWrap, bool modified) const bool emitSignals = (value != correctedValue); value = correctedValue; - updateDisplayText(modified); + updateDisplayText(); updateUpEnabled(); updateDownEnabled(); @@ -222,35 +214,46 @@ int QQuickSpinBoxPrivate::effectiveStepSize() const return from > to ? -1 * stepSize : stepSize; } -void QQuickSpinBoxPrivate::updateDisplayText(bool modified) +void QQuickSpinBoxPrivate::updateDisplayText() { - Q_Q(QQuickSpinBox); - QString text; - QQmlEngine *engine = qmlEngine(q); - if (engine && textFromValue.isCallable()) { - QJSValue loc; -#if QT_CONFIG(qml_locale) - QV4::ExecutionEngine *v4 = QQmlEnginePrivate::getV4Engine(engine); - loc = QJSValuePrivate::fromReturnedValue(QQmlLocale::wrap(v4, locale)); -#endif - text = textFromValue.call(QJSValueList() << value << loc).toString(); - } else { - text = locale.toString(value); - } - setDisplayText(text, modified); + setDisplayText(evaluateTextFromValue(value)); } -void QQuickSpinBoxPrivate::setDisplayText(const QString &text, bool modified) +void QQuickSpinBoxPrivate::setDisplayText(const QString &text) { Q_Q(QQuickSpinBox); - if (!modified && displayText == text) + if (displayText == text) return; displayText = text; emit q->displayTextChanged(); } +void QQuickSpinBoxPrivate::contentItemTextChanged() +{ + Q_Q(QQuickSpinBox); + + QQuickTextInput *inputTextItem = qobject_cast(q->contentItem()); + if (!inputTextItem) + return; + QString text = inputTextItem->text(); + validator->fixup(text); + + if (live) { + const int enteredVal = evaluateValueFromText(text); + const int correctedValue = boundValue(enteredVal, false); + if (correctedValue == enteredVal && correctedValue != value) { + // If live is true and the text is valid change the value + // setValue will set the displayText for us. + q->setValue(correctedValue); + return; + } + } + // If live is false or the value is not valid, just set the displayText + setDisplayText(text); +} + bool QQuickSpinBoxPrivate::upEnabled() const { const QQuickItem *upIndicator = up->indicator(); @@ -407,6 +410,44 @@ void QQuickSpinBoxPrivate::itemImplicitHeightChanged(QQuickItem *item) emit down->implicitIndicatorHeightChanged(); } + +QString QQuickSpinBoxPrivate::evaluateTextFromValue(int val) const +{ + Q_Q(const QQuickSpinBox); + + QString text; + QQmlEngine *engine = qmlEngine(q); + if (engine && textFromValue.isCallable()) { + QJSValue loc; +#if QT_CONFIG(qml_locale) + QV4::ExecutionEngine *v4 = QQmlEnginePrivate::getV4Engine(engine); + loc = QJSValuePrivate::fromReturnedValue(QQmlLocale::wrap(v4, locale)); +#endif + text = textFromValue.call(QJSValueList() << val << loc).toString(); + } else { + text = locale.toString(val); + } + return text; +} + +int QQuickSpinBoxPrivate::evaluateValueFromText(const QString &text) const +{ + Q_Q(const QQuickSpinBox); + int value; + QQmlEngine *engine = qmlEngine(q); + if (engine && valueFromText.isCallable()) { + QJSValue loc; +#if QT_CONFIG(qml_locale) + QV4::ExecutionEngine *v4 = QQmlEnginePrivate::getV4Engine(engine); + loc = QJSValuePrivate::fromReturnedValue(QQmlLocale::wrap(v4, locale)); +#endif + value = valueFromText.call(QJSValueList() << text << loc).toInt(); + } else { + value = locale.toInt(text); + } + return value; +} + QQuickSpinBox::QQuickSpinBox(QQuickItem *parent) : QQuickControl(*(new QQuickSpinBoxPrivate), parent) { @@ -560,6 +601,41 @@ void QQuickSpinBox::setEditable(bool editable) emit editableChanged(); } +/*! + \qmlproperty bool QtQuick.Controls::SpinBox::live + \since 6.6 + + This property holds whether the \l value is updated when the user edits the + \l displayText. The default value is \c false. If this property is \c true and + the value entered by the user is valid and within the bounds of the spinbox + [\l from, \l to], the value of the SpinBox will be set. If this property is + \c false or the value entered by the user is outside the boundaries, the + value will not be updated until the enter or return keys are pressed, or the + input field loses focus. + + \sa editable, displayText +*/ +bool QQuickSpinBox::isLive() const +{ + Q_D(const QQuickSpinBox); + return d->live; +} + +void QQuickSpinBox::setLive(bool live) +{ + Q_D(QQuickSpinBox); + if (d->live == live) + return; + + d->live = live; + + //make sure to update the value when changing to live + if (live) + d->contentItemTextChanged(); + + emit liveChanged(); +} + #if QT_CONFIG(validator) /*! \qmlproperty Validator QtQuick.Controls::SpinBox::validator @@ -1000,8 +1076,10 @@ void QQuickSpinBox::itemChange(ItemChange change, const ItemChangeData &value) void QQuickSpinBox::contentItemChange(QQuickItem *newItem, QQuickItem *oldItem) { Q_D(QQuickSpinBox); - if (QQuickTextInput *oldInput = qobject_cast(oldItem)) + if (QQuickTextInput *oldInput = qobject_cast(oldItem)) { disconnect(oldInput, &QQuickTextInput::inputMethodComposingChanged, this, &QQuickSpinBox::inputMethodComposingChanged); + QObjectPrivate::disconnect(oldInput, &QQuickTextInput::textChanged, d, &QQuickSpinBoxPrivate::contentItemTextChanged); + } if (newItem) { newItem->setActiveFocusOnTab(true); @@ -1012,8 +1090,10 @@ void QQuickSpinBox::contentItemChange(QQuickItem *newItem, QQuickItem *oldItem) newItem->setCursor(Qt::IBeamCursor); #endif - if (QQuickTextInput *newInput = qobject_cast(newItem)) + if (QQuickTextInput *newInput = qobject_cast(newItem)) { connect(newInput, &QQuickTextInput::inputMethodComposingChanged, this, &QQuickSpinBox::inputMethodComposingChanged); + QObjectPrivate::connect(newInput, &QQuickTextInput::textChanged, d, &QQuickSpinBoxPrivate::contentItemTextChanged); + } } } diff --git a/src/quicktemplates/qquickspinbox_p.h b/src/quicktemplates/qquickspinbox_p.h index a50e77c171..a6145cb08f 100644 --- a/src/quicktemplates/qquickspinbox_p.h +++ b/src/quicktemplates/qquickspinbox_p.h @@ -32,6 +32,8 @@ class Q_QUICKTEMPLATES2_PRIVATE_EXPORT QQuickSpinBox : public QQuickControl Q_PROPERTY(int value READ value WRITE setValue NOTIFY valueChanged FINAL) Q_PROPERTY(int stepSize READ stepSize WRITE setStepSize NOTIFY stepSizeChanged FINAL) Q_PROPERTY(bool editable READ isEditable WRITE setEditable NOTIFY editableChanged FINAL) + Q_PROPERTY(bool live READ isLive WRITE setLive NOTIFY liveChanged FINAL REVISION(6, 6)) + #if QT_CONFIG(validator) Q_PROPERTY(QValidator *validator READ validator WRITE setValidator NOTIFY validatorChanged FINAL) #endif @@ -68,6 +70,9 @@ public: bool isEditable() const; void setEditable(bool editable); + bool isLive() const; + void setLive(bool live); + #if QT_CONFIG(validator) QValidator *validator() const; void setValidator(QValidator *validator); @@ -105,6 +110,7 @@ Q_SIGNALS: void valueChanged(); void stepSizeChanged(); void editableChanged(); + Q_REVISION(6, 6) void liveChanged(); #if QT_CONFIG(validator) void validatorChanged(); #endif diff --git a/tests/auto/quickcontrols/controls/data/tst_spinbox.qml b/tests/auto/quickcontrols/controls/data/tst_spinbox.qml index ac6e04cc32..f9a41b9ca6 100644 --- a/tests/auto/quickcontrols/controls/data/tst_spinbox.qml +++ b/tests/auto/quickcontrols/controls/data/tst_spinbox.qml @@ -5,6 +5,7 @@ import QtQuick import QtTest import QtQuick.Controls import QtQuick.Window +import QtQuick.Layouts TestCase { id: testCase @@ -330,25 +331,421 @@ TestCase { var valueModifiedSpy = signalSpy.createObject(control, {target: control, signalName: "valueModified"}) verify(valueModifiedSpy.valid) + var displayTextChangedSpy = signalSpy.createObject(control, {target: control, signalName: "displayTextChanged"}) + verify(displayTextChangedSpy.valid) + + + control.from = 0 + control.to = 10 + compare(control.from, 0) + compare(control.to, 10) + control.contentItem.forceActiveFocus() compare(control.contentItem.activeFocus, true) compare(control.editable, false) control.contentItem.selectAll() + compare(control.displayText, "0") keyClick(Qt.Key_5) keyClick(Qt.Key_Return) + compare(control.displayText, "0") compare(control.value, 0) compare(valueModifiedSpy.count, 0) control.editable = true compare(control.editable, true) control.contentItem.selectAll() + keyClick(Qt.Key_Backspace) keyClick(Qt.Key_5) + compare(control.displayText, "5") keyClick(Qt.Key_Return) compare(control.value, 5) compare(valueModifiedSpy.count, 1) + compare(displayTextChangedSpy.count, 2) + + keyClick(Qt.Key_0) + compare(control.displayText, "50") + compare(control.value, 5) + compare(valueModifiedSpy.count, 1) + compare(displayTextChangedSpy.count, 3) + keyClick(Qt.Key_Return) //will set the value to maximum = 10 + compare(control.displayText, "10") + compare(control.value, 10) + compare(valueModifiedSpy.count, 2) // 0->5->10 + compare(displayTextChangedSpy.count, 4) //0->5->50->10 + } + + + function test_editable_liveUpdate() { + var control = createTemporaryObject(spinBox, testCase) + verify(control) + + var valueModifiedSpy = signalSpy.createObject(control, {target: control, signalName: "valueModified"}) + verify(valueModifiedSpy.valid) + + var valueChangedSpy = signalSpy.createObject(control, {target: control, signalName: "valueChanged"}) + verify(valueChangedSpy.valid) + + var displayTextChangedSpy = signalSpy.createObject(control, {target: control, signalName: "displayTextChanged"}) + verify(displayTextChangedSpy.valid) + + control.contentItem.forceActiveFocus() + compare(control.contentItem.activeFocus, true) + + control.editable = true + control.live = true + control.from = -10 + control.to = 10 + compare(control.editable, true) + compare(control.live, true) + compare(control.from, -10) + compare(control.to, 10) + + control.contentItem.selectAll() + keyClick(Qt.Key_5) + compare(control.displayText, "5") + compare(control.value, 5) + compare(valueModifiedSpy.count, 0) + compare(valueChangedSpy.count, 1) + compare(displayTextChangedSpy.count, 1) + + keyClick(Qt.Key_0) + compare(control.displayText, "50") // do not set the value + compare(control.value, 5) // if it is out of bounds + compare(valueModifiedSpy.count, 0) + compare(valueChangedSpy.count, 1) + compare(displayTextChangedSpy.count, 2) + + + keyClick(Qt.Key_Backspace) + compare(control.displayText, "5") + compare(control.value, 5) + compare(valueModifiedSpy.count, 0) + compare(valueChangedSpy.count, 1) + compare(displayTextChangedSpy.count, 3) + + keyClick(Qt.Key_Backspace) + compare(control.displayText, "0") + compare(control.value, 0) + compare(valueModifiedSpy.count, 0) + compare(valueChangedSpy.count, 2) + compare(displayTextChangedSpy.count, 4) + + keyClick(Qt.Key_Backspace) + compare(control.displayText, "") + compare(control.value, 0) + compare(valueModifiedSpy.count, 0) + compare(valueChangedSpy.count, 2) + compare(displayTextChangedSpy.count, 5) + } + + Component { + id: doubleBox + SpinBox { + id: doubleSpinBox + + property int decimals: 2 + property double realValue: value / 10**decimals + validator: DoubleValidator { + bottom: Math.min(doubleSpinBox.from, doubleSpinBox.to) + top: Math.max(doubleSpinBox.from, doubleSpinBox.to) + decimals: doubleSpinBox.decimals + notation: DoubleValidator.StandardNotation + } + + textFromValue: function(value, locale) { + let res = Number(value / 10**doubleSpinBox.decimals).toLocaleString(locale, 'f', doubleSpinBox.decimals) + return res + } + + valueFromText: function(text, locale) { + let res = Math.round(Number.fromLocaleString(locale, text) * 10**doubleSpinBox.decimals) + return res + } + } + } + + function test_editable_doubleSpinBox() { + var control = createTemporaryObject(doubleBox, testCase) + verify(control) + + var valueModifiedSpy = signalSpy.createObject(control, {target: control, signalName: "valueModified"}) + verify(valueModifiedSpy.valid) + + var valueChangedSpy = signalSpy.createObject(control, {target: control, signalName: "valueChanged"}) + verify(valueChangedSpy.valid) + + var displayTextChangedSpy = signalSpy.createObject(control, {target: control, signalName: "displayTextChanged"}) + verify(displayTextChangedSpy.valid) + + control.locale = Qt.locale("en_EN") + control.editable = true + control.from = 0 + control.to = 1000000 + control.value = 500 + control.stepSize = 1 + + compare(control.editable, true) + compare(control.from, 0) + compare(control.to, 1000000) + compare(control.value, 500) + compare(control.realValue, 5.00) + compare(control.displayText, "5.00") + + control.contentItem.forceActiveFocus() + compare(control.contentItem.activeFocus, true) + + control.contentItem.selectAll() + keyClick(Qt.Key_4) + compare(control.value, 500) + compare(control.realValue, 5.00) + compare(control.displayText, "4") + compare(valueModifiedSpy.count, 0) + compare(valueChangedSpy.count, 1) + compare(displayTextChangedSpy.count, 2) + + keyClick(Qt.Key_Enter) + compare(control.value, 400) + compare(control.realValue, 4.00) + compare(control.displayText, "4.00") + compare(valueModifiedSpy.count, 1) + compare(valueChangedSpy.count, 2) + compare(displayTextChangedSpy.count, 3) + + keyClick(Qt.Key_Backspace) + compare(control.value, 400) + compare(control.realValue, 4.00) + compare(control.displayText, "4.0") + compare(valueModifiedSpy.count, 1) + compare(valueChangedSpy.count, 2) + compare(displayTextChangedSpy.count, 4) + + keyClick(Qt.Key_Backspace) + compare(control.value, 400) + compare(control.realValue, 4.00) + compare(control.displayText, "4") //The fixup removes the trailing "." + compare(valueModifiedSpy.count, 1) + compare(valueChangedSpy.count, 2) + compare(displayTextChangedSpy.count, 5) + + keyClick(Qt.Key_0) + compare(control.value, 400) + compare(control.realValue, 4.00) + compare(control.displayText, "40") + compare(valueModifiedSpy.count, 1) + compare(valueChangedSpy.count, 2) + compare(displayTextChangedSpy.count, 6) + + keyClick(Qt.Key_0) + compare(control.value, 400) + compare(control.realValue, 4.00) + compare(control.displayText, "400") + compare(valueModifiedSpy.count, 1) + compare(valueChangedSpy.count, 2) + compare(displayTextChangedSpy.count, 7) + + keyClick(Qt.Key_0) + compare(control.value, 400) + compare(control.realValue, 4.00) + compare(control.displayText, "4,000") + compare(valueModifiedSpy.count, 1) + compare(valueChangedSpy.count, 2) + compare(displayTextChangedSpy.count, 8) + + keyClick(Qt.Key_Enter) + compare(control.value, 400000) + compare(control.realValue, 4000.00) + compare(control.displayText, "4,000.00") + compare(valueModifiedSpy.count, 2) + compare(valueChangedSpy.count, 3) + compare(displayTextChangedSpy.count, 9) + + // Changing to and testing live mode + control.live = true + compare(control.live, true) + + keyClick(Qt.Key_Backspace) + compare(control.value, 400000) + compare(control.realValue, 4000.00) + compare(control.displayText, "4,000.0") + compare(valueModifiedSpy.count, 2) + compare(valueChangedSpy.count, 3) + compare(displayTextChangedSpy.count, 10) + + keyClick(Qt.Key_Backspace) + compare(control.value, 400000) + compare(control.realValue, 4000.00) + compare(control.displayText, "4,000") //The fixup removes the trailing "." + compare(valueModifiedSpy.count, 2) + compare(valueChangedSpy.count, 3) + compare(displayTextChangedSpy.count, 11) + + keyClick(Qt.Key_Backspace) + compare(control.displayText, "400.00") + compare(control.value, 40000) + compare(control.realValue, 400.00) + compare(valueModifiedSpy.count, 2) + compare(valueChangedSpy.count, 4) + compare(displayTextChangedSpy.count, 12) + + // It is a bit unfortunate that we need 3 Backspace to go from + // 400 to 4000 on live editing mode. Maybe think about a fix in + // the future to make it more user friendly + keyClick(Qt.Key_Backspace) + keyClick(Qt.Key_Backspace) + keyClick(Qt.Key_Backspace) + compare(control.displayText, "40.00") + compare(control.value, 4000) + compare(control.realValue, 40.00) + compare(valueModifiedSpy.count, 2) + compare(valueChangedSpy.count, 5) + compare(displayTextChangedSpy.count, 15) + + keyClick(Qt.Key_Backspace) + keyClick(Qt.Key_Backspace) + keyClick(Qt.Key_Backspace) + compare(control.displayText, "4.00") + compare(control.value, 400) + compare(control.realValue, 4.00) + compare(valueModifiedSpy.count, 2) + compare(valueChangedSpy.count, 6) + compare(displayTextChangedSpy.count, 18) + + keyClick(Qt.Key_Backspace) + keyClick(Qt.Key_Backspace) + keyClick(Qt.Key_1) + compare(control.displayText, "41.00") + compare(control.value, 4100) + compare(control.realValue, 41.00) + compare(valueModifiedSpy.count, 2) + compare(valueChangedSpy.count, 7) + compare(displayTextChangedSpy.count, 21) + } + + function test_groupSeparatorHandling_data() { + return [ + { tag: "en_EN" }, + { tag: "de_DE" } + ] + } + + function test_groupSeparatorHandling(data) { + var control = createTemporaryObject(spinBox, testCase) + verify(control) + + let testLoc = Qt.locale(data.tag) + control.locale = testLoc + + control.contentItem.forceActiveFocus() + compare(control.contentItem.activeFocus, true) + + control.editable = true + control.live = true + control.from = 0 + control.to = 10*1000*1000 + compare(control.editable, true) + compare(control.live, true) + compare(control.from, 0) + compare(control.to, 10*1000*1000) + + control.contentItem.selectAll() + keyClick(Qt.Key_5) + compare(control.displayText, "5") + compare(control.value, 5) + + let i = 50 + for (; i < 1e7; i*=10) { + keyClick(Qt.Key_0) + compare(control.displayText, testLoc.toString(i)) + compare(control.value, i) + } + + i /= 100; + for (; i > 10; i/=10) { + keyClick(Qt.Key_Backspace) + compare(control.displayText, testLoc.toString(i)) + compare(control.value, i) + } } + function test_qtbug64151() { + // Slightly modified example from QTBUG-64151. We use displayText + // instead of contentItem.text as a workaround. + var control = createTemporaryObject(spinBox, testCase) + verify(control) + + control.locale = Qt.locale("en_EN") + + control.from = 0 + control.to = 2000 + control.value = 2000 + control.editable = true + + compare(control.displayText, "2,000") + + control.contentItem.forceActiveFocus() + compare(control.contentItem.activeFocus, true) + keyClick(Qt.Key_Backspace) + keyClick(Qt.Key_Return) + + compare(control.displayText, "200") + compare(control.valueFromText(control.contentItem.text, control.locale), 200) + compare(control.value, 200) + + control.contentItem.forceActiveFocus() + keyClick(Qt.Key_0) + keyClick(Qt.Key_Return) + compare(control.displayText, "2,000") + } + + Component { + id: spinBoxAndAction + RowLayout { + id: layout + property alias spinbox: theSpinbox + property alias button: theButton + SpinBox { + id: theSpinbox + from: 0 + to: 200 + value: 200 + editable: true + live: true + } + + Button { + id: theButton + property int value: 0 + action: Action { + text: "&Do something" + shortcut: "Return" + onTriggered: { + theButton.value = theSpinbox.value; + } + } + } + } + } + + function test_qtbug103205() { + + var control = createTemporaryObject(spinBoxAndAction, testCase) + verify(control) + verify(control.spinbox) + + compare(control.spinbox.displayText, "200") + control.forceActiveFocus() + control.spinbox.forceActiveFocus() + control.spinbox.contentItem.forceActiveFocus() + compare(control.spinbox.contentItem.activeFocus, true) + keyClick(Qt.Key_Backspace) + keyClick(Qt.Key_Return) + + compare(control.spinbox.displayText, "20") + compare(control.button.value, 20) + } + + function test_wheel_data() { return [ { tag: "1", properties: { from: 1, to: 10, value: 1, stepSize: 1 }, upSteps: [2,3,4], downSteps: [3,2,1,1] }, -- cgit v1.2.1