2 files changed, 59 insertions, 10 deletions

diff --git a/Libraries/LibGUI/ComboBox.cpp b/Libraries/LibGUI/ComboBox.cpp
index 490fe0dea7..538f132820 100644
--- a/Libraries/LibGUI/ComboBox.cpp
+++ b/Libraries/LibGUI/ComboBox.cpp
@@ -68,19 +68,21 @@ ComboBox::ComboBox()
             on_return_pressed();
     };
     m_editor->on_up_pressed = [this] {
-        m_list_view->move_cursor(AbstractView::CursorMovement::Up, AbstractView::SelectionUpdate::Set);
+        navigate(AbstractView::CursorMovement::Up);
     };
     m_editor->on_down_pressed = [this] {
-        m_list_view->move_cursor(AbstractView::CursorMovement::Down, AbstractView::SelectionUpdate::Set);
+        navigate(AbstractView::CursorMovement::Down);
     };
     m_editor->on_pageup_pressed = [this] {
-        m_list_view->move_cursor(AbstractView::CursorMovement::PageUp, AbstractView::SelectionUpdate::Set);
+        navigate(AbstractView::CursorMovement::PageUp);
     };
     m_editor->on_pagedown_pressed = [this] {
-        m_list_view->move_cursor(AbstractView::CursorMovement::PageDown, AbstractView::SelectionUpdate::Set);
+        navigate(AbstractView::CursorMovement::PageDown);
     };
     m_editor->on_mousewheel = [this](int delta) {
-        m_list_view->move_cursor_relative(delta, AbstractView::SelectionUpdate::Set);
+        // Since we can only show one item at a time we don't want to
+        // skip any. So just move one item at a time.
+        navigate_relative(delta > 0 ? 1 : -1);
     };
     m_editor->on_mousedown = [this] {
         if (only_allow_values_from_model())
@@ -116,14 +118,13 @@ ComboBox::ComboBox()
     m_list_view->on_selection = [this](auto& index) {
         ASSERT(model());
         m_list_view->set_activates_on_selection(true);
-        auto new_value = index.data().to_string();
-        m_editor->set_text(new_value);
-        if (!m_only_allow_values_from_model)
-            m_editor->select_all();
+        if (m_updating_model)
+            selection_updated(index);
     };
 
     m_list_view->on_activation = [this](auto& index) {
         deferred_invoke([this, index](auto&) {
+            selection_updated(index);
             if (on_change)
                 on_change(m_editor->text(), index);
         });
@@ -140,6 +141,38 @@ ComboBox::~ComboBox()
 {
 }
 
+void ComboBox::navigate(AbstractView::CursorMovement cursor_movement)
+{
+    auto previous_selected = m_list_view->cursor_index();
+    m_list_view->move_cursor(cursor_movement, AbstractView::SelectionUpdate::Set);
+    auto current_selected = m_list_view->cursor_index();
+    selection_updated(current_selected);
+    if (previous_selected.row() != current_selected.row() && on_change)
+        on_change(m_editor->text(), current_selected);
+}
+
+void ComboBox::navigate_relative(int delta)
+{
+    auto previous_selected = m_list_view->cursor_index();
+    m_list_view->move_cursor_relative(delta, AbstractView::SelectionUpdate::Set);
+    auto current_selected = m_list_view->cursor_index();
+    selection_updated(current_selected);
+    if (previous_selected.row() != current_selected.row() && on_change)
+        on_change(m_editor->text(), current_selected);
+}
+
+void ComboBox::selection_updated(const ModelIndex& index)
+{
+    if (index.is_valid())
+        m_selected_index = index;
+    else
+        m_selected_index.clear();
+    auto new_value = index.data().to_string();
+    m_editor->set_text(new_value);
+    if (!m_only_allow_values_from_model)
+        m_editor->select_all();
+}
+
 void ComboBox::resize_event(ResizeEvent& event)
 {
     Widget::resize_event(event);
@@ -153,6 +186,8 @@ void ComboBox::resize_event(ResizeEvent& event)
 
 void ComboBox::set_model(NonnullRefPtr<Model> model)
 {
+    TemporaryChange change(m_updating_model, true);
+    m_selected_index.clear();
     m_list_view->set_model(move(model));
 }
 
@@ -160,12 +195,13 @@ void ComboBox::set_selected_index(size_t index)
 {
     if (!m_list_view->model())
         return;
+    TemporaryChange change(m_updating_model, true);
     m_list_view->set_cursor(m_list_view->model()->index(index, 0), AbstractView::SelectionUpdate::Set);
 }
 
 size_t ComboBox::selected_index() const
 {
-    return m_list_view->cursor_index().row();
+    return m_selected_index.has_value() ? m_selected_index.value().row() : 0;
 }
 
 void ComboBox::select_all()
@@ -201,6 +237,11 @@ void ComboBox::open()
 
     m_editor->set_has_visible_list(true);
     m_editor->set_focus(true);
+    if (m_selected_index.has_value()) {
+        // Don't set m_updating_model to true here because we only want to
+        // change the list view's selected item without triggering a change to it.
+        m_list_view->set_cursor(m_selected_index.value(), AbstractView::SelectionUpdate::Set);
+    }
     m_list_window->set_rect(list_window_rect);
     m_list_window->show();
 }
diff --git a/Libraries/LibGUI/ComboBox.h b/Libraries/LibGUI/ComboBox.h
index a7e432fef7..79ab26b7e3 100644
--- a/Libraries/LibGUI/ComboBox.h
+++ b/Libraries/LibGUI/ComboBox.h
@@ -26,7 +26,9 @@
 
 #pragma once
 
+#include <LibGUI/AbstractView.h>
 #include <LibGUI/Frame.h>
+#include <LibGUI/Model.h>
 
 namespace GUI {
 
@@ -67,11 +69,17 @@ protected:
     virtual void resize_event(ResizeEvent&) override;
 
 private:
+    void selection_updated(const ModelIndex&);
+    void navigate(AbstractView::CursorMovement);
+    void navigate_relative(int);
+
     RefPtr<ComboBoxEditor> m_editor;
     RefPtr<ControlBoxButton> m_open_button;
     RefPtr<Window> m_list_window;
     RefPtr<ListView> m_list_view;
+    Optional<ModelIndex> m_selected_index;
     bool m_only_allow_values_from_model { false };
+    bool m_updating_model { false };
 };
 
 }