QQuickComboBox: update highlighted index on mouse hover

This is the expected behavior on desktop.

NOTE: We can no longer rely on ListView.ApplyRange, because if ListView
adjusts the content position while key navigating or scrolling, "wrong"
items may get hovered and the highlighted index gets set incorrectly
leading to a hovered->highlight->hovered loop. Therefore we force
ListView.NoHighlightRange to keep existing styles working without
modifications.

Change-Id: I57fe3de1230dd6348d01c1785cd09d4fb184d28a
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

5 files changed, 92 insertions, 4 deletions

diff --git a/src/imports/controls/ComboBox.qml b/src/imports/controls/ComboBox.qml
index 162a799a..8442ec79 100644
--- a/src/imports/controls/ComboBox.qml
+++ b/src/imports/controls/ComboBox.qml
@@ -120,7 +120,6 @@ T.ComboBox {
             implicitHeight: contentHeight
             model: control.popup.visible ? control.delegateModel : null
             currentIndex: control.highlightedIndex
-            highlightRangeMode: ListView.ApplyRange
             highlightMoveDuration: 0
 
             Rectangle {
diff --git a/src/imports/controls/material/ComboBox.qml b/src/imports/controls/material/ComboBox.qml
index c2bdff65..55afdb3f 100644
--- a/src/imports/controls/material/ComboBox.qml
+++ b/src/imports/controls/material/ComboBox.qml
@@ -162,7 +162,6 @@ T.ComboBox {
             implicitHeight: contentHeight
             model: control.popup.visible ? control.delegateModel : null
             currentIndex: control.highlightedIndex
-            highlightRangeMode: ListView.ApplyRange
             highlightMoveDuration: 0
 
             T.ScrollIndicator.vertical: ScrollIndicator { }
diff --git a/src/imports/controls/universal/ComboBox.qml b/src/imports/controls/universal/ComboBox.qml
index 5ec5bbee..35c4d815 100644
--- a/src/imports/controls/universal/ComboBox.qml
+++ b/src/imports/controls/universal/ComboBox.qml
@@ -144,7 +144,6 @@ T.ComboBox {
             implicitHeight: contentHeight
             model: control.popup.visible ? control.delegateModel : null
             currentIndex: control.highlightedIndex
-            highlightRangeMode: ListView.ApplyRange
             highlightMoveDuration: 0
 
             T.ScrollIndicator.vertical: ScrollIndicator { }
diff --git a/src/quicktemplates2/qquickcombobox.cpp b/src/quicktemplates2/qquickcombobox.cpp
index 3f0d359e..f511045a 100644
--- a/src/quicktemplates2/qquickcombobox.cpp
+++ b/src/quicktemplates2/qquickcombobox.cpp
@@ -37,6 +37,7 @@
 #include "qquickcombobox_p.h"
 #include "qquickcontrol_p_p.h"
 #include "qquickabstractbutton_p.h"
+#include "qquickabstractbutton_p_p.h"
 #include "qquickpopup_p_p.h"
 
 #include <QtCore/qregexp.h>
@@ -50,6 +51,7 @@
 #include <QtQml/private/qqmldelegatemodel_p.h>
 #include <QtQuick/private/qquickevents_p_p.h>
 #include <QtQuick/private/qquicktextinput_p.h>
+#include <QtQuick/private/qquickitemview_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -221,6 +223,7 @@ public:
     void popupVisibleChanged();
 
     void itemClicked();
+    void itemHovered();
 
     void createdItem(int index, QObject *object);
     void modelUpdated();
@@ -253,6 +256,7 @@ public:
     bool hasDown;
     bool pressed;
     bool ownModel;
+    bool keyNavigating;
     bool hasDisplayText;
     bool hasCurrentIndex;
     int highlightedIndex;
@@ -291,6 +295,7 @@ QQuickComboBoxPrivate::QQuickComboBoxPrivate()
       hasDown(false),
       pressed(false),
       ownModel(false),
+      keyNavigating(false),
       hasDisplayText(false),
       hasCurrentIndex(false),
       highlightedIndex(-1),
@@ -341,7 +346,15 @@ void QQuickComboBoxPrivate::popupVisibleChanged()
     if (isPopupVisible())
         QGuiApplication::inputMethod()->reset();
 
+    QQuickItemView *itemView = popup->findChild<QQuickItemView *>();
+    if (itemView)
+        itemView->setHighlightRangeMode(QQuickItemView::NoHighlightRange);
+
     updateHighlightedIndex();
+
+    if (itemView)
+        itemView->positionViewAtIndex(highlightedIndex, QQuickItemView::Beginning);
+
     if (!hasDown) {
         q->setDown(pressed || isPopupVisible());
         hasDown = false;
@@ -358,6 +371,25 @@ void QQuickComboBoxPrivate::itemClicked()
     }
 }
 
+void QQuickComboBoxPrivate::itemHovered()
+{
+    Q_Q(QQuickComboBox);
+    if (keyNavigating)
+        return;
+
+    QQuickAbstractButton *button = qobject_cast<QQuickAbstractButton *>(q->sender());
+    if (!button || !button->isHovered() || QQuickAbstractButtonPrivate::get(button)->touchId != -1)
+        return;
+
+    int index = delegateModel->indexOf(button, nullptr);
+    if (index != -1) {
+        setHighlightedIndex(index, Highlight);
+
+        if (QQuickItemView *itemView = popup->findChild<QQuickItemView *>())
+            itemView->positionViewAtIndex(index, QQuickItemView::Contain);
+    }
+}
+
 void QQuickComboBoxPrivate::createdItem(int index, QObject *object)
 {
     Q_Q(QQuickComboBox);
@@ -371,6 +403,7 @@ void QQuickComboBoxPrivate::createdItem(int index, QObject *object)
     if (button) {
         button->setFocusPolicy(Qt::NoFocus);
         connect(button, &QQuickAbstractButton::clicked, this, &QQuickComboBoxPrivate::itemClicked);
+        connect(button, &QQuickAbstractButton::hoveredChanged, this, &QQuickComboBoxPrivate::itemHovered);
     }
 
     if (index == currentIndex && !q->isEditable())
@@ -1155,6 +1188,9 @@ void QQuickComboBox::setPopup(QQuickPopup *popup)
         QQuickPopupPrivate::get(popup)->allowVerticalFlip = true;
         popup->setClosePolicy(QQuickPopup::CloseOnEscape | QQuickPopup::CloseOnPressOutsideParent);
         QObjectPrivate::connect(popup, &QQuickPopup::visibleChanged, d, &QQuickComboBoxPrivate::popupVisibleChanged);
+
+        if (QQuickItemView *itemView = popup->findChild<QQuickItemView *>())
+            itemView->setHighlightRangeMode(QQuickItemView::NoHighlightRange);
     }
     d->popup = popup;
     emit popupChanged();
@@ -1446,14 +1482,17 @@ void QQuickComboBox::keyPressEvent(QKeyEvent *event)
         event->accept();
         break;
     case Qt::Key_Up:
+        d->keyNavigating = true;
         d->decrementCurrentIndex();
         event->accept();
         break;
     case Qt::Key_Down:
+        d->keyNavigating = true;
         d->incrementCurrentIndex();
         event->accept();
         break;
     case Qt::Key_Home:
+        d->keyNavigating = true;
         if (d->isPopupVisible())
             d->setHighlightedIndex(0, Highlight);
         else
@@ -1461,6 +1500,7 @@ void QQuickComboBox::keyPressEvent(QKeyEvent *event)
         event->accept();
         break;
     case Qt::Key_End:
+        d->keyNavigating = true;
         if (d->isPopupVisible())
             d->setHighlightedIndex(count() - 1, Highlight);
         else
@@ -1480,6 +1520,8 @@ void QQuickComboBox::keyReleaseEvent(QKeyEvent *event)
 {
     Q_D(QQuickComboBox);
     QQuickControl::keyReleaseEvent(event);
+
+    d->keyNavigating = false;
     if (!d->popup || event->isAutoRepeat())
         return;
 
diff --git a/tests/auto/controls/data/tst_combobox.qml b/tests/auto/controls/data/tst_combobox.qml
index 3d8777b3..39a1d145 100644
--- a/tests/auto/controls/data/tst_combobox.qml
+++ b/tests/auto/controls/data/tst_combobox.qml
@@ -699,7 +699,7 @@ TestCase {
         compare(highlightedSpy.count, 0)
         mouseMove(content, content.width / 2 + 1, content.height / 2 + 1)
         compare(activatedSpy.count, 0)
-        compare(highlightedSpy.count, 0)
+        compare(highlightedSpy.count, 1)
         mouseRelease(content)
         compare(activatedSpy.count, 1)
         compare(highlightedSpy.count, 1)
@@ -1143,6 +1143,55 @@ TestCase {
         closedSpy.target = null
     }
 
+    function test_mouseHighlight() {
+        var control = createTemporaryObject(comboBox, testCase, {model: 20})
+        verify(control)
+
+        compare(control.highlightedIndex, -1)
+
+        var openedSpy = signalSpy.createObject(control, {target: control.popup, signalName: "opened"})
+        verify(openedSpy.valid)
+
+        control.popup.open()
+        compare(control.highlightedIndex, 0)
+        tryCompare(openedSpy, "count", 1)
+
+        var listview = control.popup.contentItem
+        verify(listview)
+        waitForRendering(listview)
+
+        // hover-highlight through all visible list items one by one
+        var hoverIndex = -1
+        var prevHoverItem = null
+        for (var y = 0; y < listview.height; ++y) {
+            var hoverItem = listview.itemAt(0, listview.contentY + y)
+            if (!hoverItem || !hoverItem.visible || hoverItem === prevHoverItem)
+                continue
+            mouseMove(hoverItem, 0, 0)
+            tryCompare(control, "highlightedIndex", ++hoverIndex)
+            prevHoverItem = hoverItem
+        }
+
+        mouseMove(listview, listview.width / 2, listview.height / 2)
+
+        // wheel-highlight the rest of the items
+        var delta = 120
+        var prevWheelItem = null
+        while (!listview.atYEnd) {
+            var prevContentY = listview.contentY
+            mouseWheel(listview, listview.width / 2, listview.height / 2, -delta, -delta)
+            tryCompare(listview, "moving", false)
+            verify(listview.contentY > prevContentY)
+
+            var wheelItem = listview.itemAt(listview.width / 2, listview.contentY + listview.height / 2)
+            if (!wheelItem || !wheelItem.visible || wheelItem === prevWheelItem)
+                continue
+
+            tryCompare(control, "highlightedIndex", parseInt(wheelItem.text))
+            prevWheelItem = wheelItem
+        }
+    }
+
     RegExpValidator {
         id: regExpValidator
         regExp: /(red|blue|green)?/