Tumbler: make wrap property depend on count by default

[ChangeLog][Important Behavior Changes][Tumbler] Changed the default
value of wrap to be false when count is less than visibleItemCount.
Explicitly setting wrap overrides this behavior.

Change-Id: I0089f517a25a606625c245df52b0db5fd859ffc0
Task-number: QTBUG-53587
Reviewed-by: J-P Nurmi <jpnurmi@qt.io>

1 files changed, 195 insertions, 116 deletions

diff --git a/src/quicktemplates2/qquicktumbler.cpp b/src/quicktemplates2/qquicktumbler.cpp
index 1d3c3240..49f3fa42 100644
--- a/src/quicktemplates2/qquicktumbler.cpp
+++ b/src/quicktemplates2/qquicktumbler.cpp
@@ -38,6 +38,7 @@
 
 #include <QtQuick/private/qquickflickable_p.h>
 #include <QtQuickTemplates2/private/qquickcontrol_p_p.h>
+#include <QtQuickTemplates2/private/qquicktumbler_p_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -61,8 +62,7 @@ QT_BEGIN_NAMESPACE
     items. It is useful for when there are too many options to use, for
     example, a RadioButton, and too few options to require the use of an
     editable SpinBox. It is convenient in that it requires no keyboard usage
-    and can be made to wrap around at each end when there are a large number of
-    items.
+    and wraps around at each end when there are a large number of items.
 
     The API is similar to that of views like \l ListView and \l PathView; a
     \l model and \l delegate can be set, and the \l count and \l currentItem
@@ -73,8 +73,9 @@ QT_BEGIN_NAMESPACE
     equal to \c 0, \l currentIndex will be \c -1. In all other cases, it will
     be greater than or equal to \c 0.
 
-    By default, Tumbler wraps when it reaches the top and bottom. To achieve a
-    non-wrapping Tumbler, set the \l wrap property to \c false:
+    By default, Tumbler \l {wrap}{wraps} when it reaches the top and bottom, as
+    long as there are more items in the model than there are visible items;
+    that is, when \l count is greater than \l visibleItemCount:
 
     \snippet qtquickcontrols2-tumbler-timePicker.qml tumbler
 
@@ -83,64 +84,25 @@ QT_BEGIN_NAMESPACE
     \sa {Customizing Tumbler}, {Input Controls}
 */
 
-class QQuickTumblerPrivate : public QQuickControlPrivate, public QQuickItemChangeListener
+QQuickTumblerPrivate::QQuickTumblerPrivate() :
+    delegate(nullptr),
+    visibleItemCount(5),
+    wrap(true),
+    explicitWrap(false),
+    ignoreWrapChanges(false),
+    view(nullptr),
+    viewContentItem(nullptr),
+    viewContentItemType(UnsupportedContentItemType),
+    currentIndex(-1),
+    pendingCurrentIndex(-1),
+    ignoreCurrentIndexChanges(false),
+    count(0)
 {
-    Q_DECLARE_PUBLIC(QQuickTumbler)
-
-public:
-    QQuickTumblerPrivate() :
-        delegate(nullptr),
-        visibleItemCount(5),
-        wrap(true),
-        view(nullptr),
-        viewContentItem(nullptr),
-        viewContentItemType(UnsupportedContentItemType),
-        currentIndex(-1),
-        ignoreCurrentIndexChanges(false)
-    {
-    }
-
-    ~QQuickTumblerPrivate()
-    {
-    }
-
-    enum ContentItemType {
-        UnsupportedContentItemType,
-        PathViewContentItem,
-        ListViewContentItem
-    };
-
-    QQuickItem *determineViewType(QQuickItem *contentItem);
-    void resetViewData();
-    QList<QQuickItem *> viewContentItemChildItems() const;
-
-    static QQuickTumblerPrivate *get(QQuickTumbler *tumbler)
-    {
-        return tumbler->d_func();
-    }
-
-    QVariant model;
-    QQmlComponent *delegate;
-    int visibleItemCount;
-    bool wrap;
-    QQuickItem *view;
-    QQuickItem *viewContentItem;
-    ContentItemType viewContentItemType;
-    int currentIndex;
-    bool ignoreCurrentIndexChanges;
-
-    void _q_updateItemHeights();
-    void _q_updateItemWidths();
-    void _q_onViewCurrentIndexChanged();
-    void _q_onViewCountChanged();
-
-    void disconnectFromView();
-    void setupViewData(QQuickItem *newControlContentItem);
-    void syncCurrentIndex();
+}
 
-    void itemChildAdded(QQuickItem *, QQuickItem *) override;
-    void itemChildRemoved(QQuickItem *, QQuickItem *) override;
-};
+QQuickTumblerPrivate::~QQuickTumblerPrivate()
+{
+}
 
 namespace {
     static inline qreal delegateHeight(const QQuickTumbler *tumbler)
@@ -193,6 +155,11 @@ QList<QQuickItem *> QQuickTumblerPrivate::viewContentItemChildItems() const
     return viewContentItem->childItems();
 }
 
+QQuickTumblerPrivate *QQuickTumblerPrivate::get(QQuickTumbler *tumbler)
+{
+    return tumbler->d_func();
+}
+
 void QQuickTumblerPrivate::_q_updateItemHeights()
 {
     // Can't use our own private padding members here, as the padding property might be set,
@@ -219,9 +186,7 @@ void QQuickTumblerPrivate::_q_onViewCurrentIndexChanged()
     if (!ignoreCurrentIndexChanges) {
         Q_ASSERT(view);
         const int oldCurrentIndex = currentIndex;
-
         currentIndex = view->property("currentIndex").toInt();
-
         if (oldCurrentIndex != currentIndex)
             emit q->currentIndexChanged();
     }
@@ -230,14 +195,28 @@ void QQuickTumblerPrivate::_q_onViewCurrentIndexChanged()
 void QQuickTumblerPrivate::_q_onViewCountChanged()
 {
     Q_Q(QQuickTumbler);
-    // If new items were added and our currentIndex was -1, we must
-    // enforce our rule of a non-negative currentIndex when count > 0.
-    if (q->count() > 0 && currentIndex == -1)
-        q->setCurrentIndex(0);
-    else
-        syncCurrentIndex();
 
-    emit q->countChanged();
+    setCount(view->property("count").toInt());
+
+    if (count > 0) {
+        if (pendingCurrentIndex != -1) {
+            // If there was an attempt to set currentIndex at creation, try to finish that attempt now.
+            // componentComplete() is too early, because the count might only be known sometime after completion.
+            q->setCurrentIndex(pendingCurrentIndex);
+            // If we could successfully set the currentIndex, consider it done.
+            // Otherwise, we'll try again later in updatePolish().
+            if (currentIndex == pendingCurrentIndex)
+                pendingCurrentIndex = -1;
+            else
+                q->polish();
+        } else if (currentIndex == -1) {
+            // If new items were added and our currentIndex was -1, we must
+            // enforce our rule of a non-negative currentIndex when count > 0.
+            q->setCurrentIndex(0);
+        }
+    } else {
+        q->setCurrentIndex(-1);
+    }
 }
 
 void QQuickTumblerPrivate::itemChildAdded(QQuickItem *, QQuickItem *)
@@ -287,9 +266,13 @@ void QQuickTumbler::setModel(const QVariant &model)
     if (model == d->model)
         return;
 
+    d->lockWrap();
+
     d->model = model;
     emit modelChanged();
 
+    d->unlockWrap();
+
     // Don't try to correct the currentIndex if count() isn't known yet.
     // We can check in setupViewData() instead.
     if (isComponentComplete() && d->view && count() == 0)
@@ -305,7 +288,7 @@ void QQuickTumbler::setModel(const QVariant &model)
 int QQuickTumbler::count() const
 {
     Q_D(const QQuickTumbler);
-    return d->view ? d->view->property("count").toInt() : 0;
+    return d->count;
 }
 
 /*!
@@ -325,25 +308,47 @@ int QQuickTumbler::currentIndex() const
 void QQuickTumbler::setCurrentIndex(int currentIndex)
 {
     Q_D(QQuickTumbler);
+    if (currentIndex == d->currentIndex || currentIndex < -1)
+        return;
+
+    if (!isComponentComplete()) {
+        // Views can't set currentIndex until they're ready.
+        d->pendingCurrentIndex = currentIndex;
+        return;
+    }
+
     // -1 doesn't make sense for a non-empty Tumbler, because unlike
     // e.g. ListView, there's always one item selected.
     // Wait until the component has finished before enforcing this rule, though,
     // because the count might not be known yet.
-    if (currentIndex == d->currentIndex || (isComponentComplete() && currentIndex == -1 && count() > 0))
+    if ((d->count > 0 && currentIndex == -1) || (currentIndex >= d->count)) {
         return;
+    }
 
-    d->currentIndex = currentIndex;
-
+    // The view might not have been created yet, as is the case
+    // if you create a Tumbler component and pass e.g. { currentIndex: 2 }
+    // to createObject().
     if (d->view) {
-        // The view might not have been created yet, as is the case
-        // if you create a Tumbler component and pass e.g. { currentIndex: 2 }
-        // to createObject().
-        d->ignoreCurrentIndexChanges = true;
-        d->view->setProperty("currentIndex", currentIndex);
-        d->ignoreCurrentIndexChanges = false;
-    }
+        // Only actually set our currentIndex if the view was able to set theirs.
+        bool couldSet = false;
+        if (d->count == 0 && currentIndex == -1) {
+            // PathView insists on using 0 as the currentIndex when there are no items.
+            couldSet = true;
+        } else {
+            d->ignoreCurrentIndexChanges = true;
+            d->view->setProperty("currentIndex", currentIndex);
+            d->ignoreCurrentIndexChanges = false;
+
+            couldSet = d->view->property("currentIndex").toInt() == currentIndex;
+        }
 
-    emit currentIndexChanged();
+        if (couldSet) {
+            // The view's currentIndex might not have actually changed, but ours has,
+            // and that's what user code sees.
+            d->currentIndex = currentIndex;
+            emit currentIndexChanged();
+        }
+    }
 }
 
 /*!
@@ -409,9 +414,11 @@ void QQuickTumbler::setVisibleItemCount(int visibleItemCount)
     This property determines whether or not the tumbler wraps around when it
     reaches the top or bottom.
 
-    It is recommended to set this property to \c false when \l count is less than
+    The default value is \c false when \l count is less than
     \l visibleItemCount, as it is simpler to interact with a non-wrapping Tumbler
-    when there are only a few items.
+    when there are only a few items. To override this behavior, explicitly set
+    the value of this property. To return to the default behavior, set this
+    property to \c undefined.
 */
 bool QQuickTumbler::wrap() const
 {
@@ -422,34 +429,14 @@ bool QQuickTumbler::wrap() const
 void QQuickTumbler::setWrap(bool wrap)
 {
     Q_D(QQuickTumbler);
-    if (isComponentComplete() && wrap == d->wrap)
-        return;
-
-    // Since we use the currentIndex of the contentItem directly, we must
-    // ensure that we keep track of the currentIndex so it doesn't get lost
-    // between view changes.
-    const int oldCurrentIndex = currentIndex();
-
-    d->disconnectFromView();
-
-    d->wrap = wrap;
-
-    // New views will set their currentIndex upon creation, which we'd otherwise
-    // take as the correct one, so we must ignore them.
-    d->ignoreCurrentIndexChanges = true;
-
-    // This will cause the view to be created if our contentItem is a TumblerView.
-    emit wrapChanged();
-
-    d->ignoreCurrentIndexChanges = false;
-
-    // The view should have been created now, so we can start determining its type, etc.
-    // If the delegates use attached properties, this will have already been called,
-    // in which case it will return early. If the delegate doesn't use attached properties,
-    // we need to call it here.
-    d->setupViewData(d->contentItem);
+    d->setWrap(wrap, true);
+}
 
-    setCurrentIndex(oldCurrentIndex);
+void QQuickTumbler::resetWrap()
+{
+    Q_D(QQuickTumbler);
+    d->explicitWrap = false;
+    d->setWrapBasedOnCount();
 }
 
 QQuickTumblerAttached *QQuickTumbler::qmlAttachedProperties(QObject *object)
@@ -483,12 +470,9 @@ void QQuickTumbler::componentComplete()
     d->_q_updateItemWidths();
 
     if (!d->view) {
-        // We don't want to create a PathView or ListView until we're certain
-        // which one we need, and if wrap is not set, it will be the default.
-        // We can only know the final value of wrap when componentComplete() is called,
-        // so, if the view hasn't already been created, we cause it to be created here.
+        // Force the view to be created.
         emit wrapChanged();
-        // Then, we determine the type of view for attached properties, etc.
+        // Determine the type of view for attached properties, etc.
         d->setupViewData(d->contentItem);
     }
 }
@@ -578,6 +562,74 @@ void QQuickTumblerPrivate::syncCurrentIndex()
     ignoreCurrentIndexChanges = false;
 }
 
+void QQuickTumblerPrivate::setCount(int newCount)
+{
+    if (newCount == count)
+        return;
+
+    count = newCount;
+
+    Q_Q(QQuickTumbler);
+    setWrapBasedOnCount();
+
+    emit q->countChanged();
+}
+
+void QQuickTumblerPrivate::setWrapBasedOnCount()
+{
+    if (count == 0 || explicitWrap || ignoreWrapChanges)
+        return;
+
+    setWrap(count >= visibleItemCount, false);
+}
+
+void QQuickTumblerPrivate::setWrap(bool shouldWrap, bool isExplicit)
+{
+    if (isExplicit)
+        explicitWrap = true;
+
+    Q_Q(QQuickTumbler);
+    if (q->isComponentComplete() && shouldWrap == wrap)
+        return;
+
+    // Since we use the currentIndex of the contentItem directly, we must
+    // ensure that we keep track of the currentIndex so it doesn't get lost
+    // between view changes.
+    const int oldCurrentIndex = currentIndex;
+
+    disconnectFromView();
+
+    wrap = shouldWrap;
+
+    // New views will set their currentIndex upon creation, which we'd otherwise
+    // take as the correct one, so we must ignore them.
+    ignoreCurrentIndexChanges = true;
+
+    // This will cause the view to be created if our contentItem is a TumblerView.
+    emit q->wrapChanged();
+
+    ignoreCurrentIndexChanges = false;
+
+    // The view should have been created now, so we can start determining its type, etc.
+    // If the delegates use attached properties, this will have already been called,
+    // in which case it will return early. If the delegate doesn't use attached properties,
+    // we need to call it here.
+    setupViewData(contentItem);
+
+    q->setCurrentIndex(oldCurrentIndex);
+}
+
+void QQuickTumblerPrivate::lockWrap()
+{
+    ignoreWrapChanges = true;
+}
+
+void QQuickTumblerPrivate::unlockWrap()
+{
+    ignoreWrapChanges = false;
+    setWrapBasedOnCount();
+}
+
 void QQuickTumbler::keyPressEvent(QKeyEvent *event)
 {
     QQuickControl::keyPressEvent(event);
@@ -593,6 +645,31 @@ void QQuickTumbler::keyPressEvent(QKeyEvent *event)
     }
 }
 
+void QQuickTumbler::updatePolish()
+{
+    Q_D(QQuickTumbler);
+    if (d->pendingCurrentIndex != -1) {
+        // If the count is still 0, it's not going to happen.
+        if (d->count == 0) {
+            d->pendingCurrentIndex = -1;
+            return;
+        }
+
+        // If there is a pending currentIndex at this stage, it means that
+        // the view wouldn't set our currentIndex in _q_onViewCountChanged
+        // because it wasn't ready. Try one last time here.
+        setCurrentIndex(d->pendingCurrentIndex);
+
+        if (d->currentIndex != d->pendingCurrentIndex && d->currentIndex == -1) {
+            // If we *still* couldn't set it, it's probably invalid.
+            // See if we can at least enforce our rule of "non-negative currentIndex when count > 0" instead.
+            setCurrentIndex(0);
+        }
+
+        d->pendingCurrentIndex = -1;
+    }
+}
+
 class QQuickTumblerAttachedPrivate : public QObjectPrivate, public QQuickItemChangeListener
 {
     Q_DECLARE_PUBLIC(QQuickTumblerAttached)
@@ -679,7 +756,9 @@ void QQuickTumblerAttachedPrivate::_q_calculateDisplacement()
     if (!tumblerPrivate->viewContentItem)
         return;
 
-    const int count = tumbler->count();
+    // The attached property gets created before our count is updated, so just cheat here
+    // to avoid having to listen to count changes.
+    const int count = tumblerPrivate->view->property("count").toInt();
     // This can happen in tests, so it may happen in normal usage too.
     if (count == 0)
         return;