Binding with combobox index was broken, as discussed in D24916
As stated in Qt https://doc.qt.io/qt-5/qml-qtquick-controls2-combobox.html#delegate-prop
When using ItemDelegate no need to explicitly close the popup and set the currentIndex.
Details
Details
Use kcm_cursortheme
2 test case
- In size combobx, change the value, hit apply, then press default, should reset combobox value to default
- Also in size combobx, press and hold the mose, hover to change the value and then release mouse button to set the new value, hit apply, then hit default, combobox value should be set to default value.
Diff Detail
Diff Detail
- Repository
- R858 Qt Quick Controls 2: Desktop Style
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
Good start, there's one case still missing... which looks hard to reach, I wonder how to get there.
| org.kde.desktop/ComboBox.qml | ||
|---|---|---|
| 92 | What about that one? :-) | |
| org.kde.desktop/ComboBox.qml | ||
|---|---|---|
| 92 | Yes, this one breaks as well. | |
Comment Actions
Remove assignement to currentIndex.
Add a C++ bypass to setCurrentIndex without breaking the binding.
Comment Actions
Almost there! Glad we're getting near a proper fix. Can you confirm this works with *and* without D24916 applied?
Can you also confirm this doesn't break the KScreen KCM? (as mentioned in Kai's comment on D24916).
I want to make sure we don't introduce a regression for those who had started to workaround that bug on their side.
| plugin/kpropertywriter.h | ||
|---|---|---|
| 1 ↗ | (On Diff #69053) | This file should be named kpropertywriter_p.h since it's not installed and the class not exported from a library. |
| 39 ↗ | (On Diff #69053) | const QVariant & |
| 43 ↗ | (On Diff #69053) | const QString & |
| 47 ↗ | (On Diff #69053) | const QString & |
| 50 ↗ | (On Diff #69053) | If you want to play like this, you could have had a "using QObject::QObject;" for the ctor declaration and have no ctor definition in the cpp file. ;-) |
Comment Actions
There's a Plasma rule that if we're working round a Qt bug, there should be a Qt bug created and linked before accepting a workaround.
From the sounds of it we want a QQuickControls::ComboBox::setIndex(int) invokable that doesn't update the binding?
Or is it a more generic problem of somehow exposing QQmlPropertyData::WriteFlags ?
| plugin/kpropertywriter_p.h | ||
|---|---|---|
| 27 | Throwing out another option class KPropertyWriter : public QObject, public QQmlPropertyValueSource
{
Q_INVOKABLE bool writeProperty(QVariant value);
}
writeProperty(QVariant) {
object()->setProperty(name(), value());
// we can't use property().write() as that'll break the binding
}
PropertyWriter on currentIndex {
id: controlRootWriter
}Though it's basically the same thing, so don't feel you have to, just wanted to share the suggestion as it reduces two properties. | |
Comment Actions
It's the former, not the latter. And good luck having setIndex invokable, it would never go through a Qt review IMHO (and for good reason). Now we need this really because our own combo style insists on emulating a corner case of QComboBox behavior, it's fairly unusual and specific.
I don't think it warrants putting something in Qt directly.
| plugin/kpropertywriter_p.h | ||
|---|---|---|
| 27 | Didn't think about that one, clever trick indeed. :-) That being said I think it'd be more work in the end, indeed setTarget is pure virtual in there, and also I'm not sure that calling write() on QQmlProperty doesn't break bindings. | |
Comment Actions
Looks good to me. Please just wait a bit before pushing to give David a chance to object to my comments. ;-)
| plugin/kpropertywriter_p.h | ||
|---|---|---|
| 27 | Thank you for sharing other option, I didn't know this one. | |