From 0f35912bd789e37b8e7193ffc88827fd1f2fcfec Mon Sep 17 00:00:00 2001
From: Jelle Raaijmakers <jelle@gmta.nl>
Date: Sun, 11 Jul 2021 21:46:34 +0200
Subject: [PATCH] TableView: Do not select input on keydown

In the Spreadsheet app, selecting a cell and typing something (like
"1") would create an empty editing delegate, set "1" as its value and
immediately select the entire contents of the text box. If your goal
was to type "123", that "1" was selected and will be replaced by "23".

This changes the behavior of TableView to not select the editing
delegate's contents if its creation was a result of a keydown event.
---
 .../Applications/Spreadsheet/SpreadsheetView.cpp     | 10 +++++-----
 Userland/Applications/Spreadsheet/SpreadsheetView.h  |  2 +-
 Userland/Libraries/LibGUI/ModelEditingDelegate.h     | 12 +++++++++---
 Userland/Libraries/LibGUI/TableView.cpp              | 12 ++++++++----
 4 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/Userland/Applications/Spreadsheet/SpreadsheetView.cpp b/Userland/Applications/Spreadsheet/SpreadsheetView.cpp
index eb239f6936..97cc81b258 100644
--- a/Userland/Applications/Spreadsheet/SpreadsheetView.cpp
+++ b/Userland/Applications/Spreadsheet/SpreadsheetView.cpp
@@ -25,23 +25,23 @@ SpreadsheetView::~SpreadsheetView()
 {
 }
 
-void SpreadsheetView::EditingDelegate::set_value(const GUI::Variant& value)
+void SpreadsheetView::EditingDelegate::set_value(GUI::Variant const& value, GUI::ModelEditingDelegate::SelectionBehavior selection_behavior)
 {
     if (value.as_string().is_null()) {
-        StringModelEditingDelegate::set_value("");
+        StringModelEditingDelegate::set_value("", selection_behavior);
         commit();
         return;
     }
 
     if (m_has_set_initial_value)
-        return StringModelEditingDelegate::set_value(value);
+        return StringModelEditingDelegate::set_value(value, selection_behavior);
 
     m_has_set_initial_value = true;
     const auto option = m_sheet.at({ (size_t)index().column(), (size_t)index().row() });
     if (option)
-        return StringModelEditingDelegate::set_value(option->source());
+        return StringModelEditingDelegate::set_value(option->source(), selection_behavior);
 
-    StringModelEditingDelegate::set_value("");
+    StringModelEditingDelegate::set_value("", selection_behavior);
 }
 
 void InfinitelyScrollableTableView::did_scroll()
diff --git a/Userland/Applications/Spreadsheet/SpreadsheetView.h b/Userland/Applications/Spreadsheet/SpreadsheetView.h
index 3c5f371fc9..38e304346a 100644
--- a/Userland/Applications/Spreadsheet/SpreadsheetView.h
+++ b/Userland/Applications/Spreadsheet/SpreadsheetView.h
@@ -116,7 +116,7 @@ private:
             : m_sheet(sheet)
         {
         }
-        virtual void set_value(const GUI::Variant& value) override;
+        virtual void set_value(GUI::Variant const&, GUI::ModelEditingDelegate::SelectionBehavior) override;
 
         virtual RefPtr<Widget> create_widget() override
         {
diff --git a/Userland/Libraries/LibGUI/ModelEditingDelegate.h b/Userland/Libraries/LibGUI/ModelEditingDelegate.h
index 2678f25cfd..a910b67b05 100644
--- a/Userland/Libraries/LibGUI/ModelEditingDelegate.h
+++ b/Userland/Libraries/LibGUI/ModelEditingDelegate.h
@@ -14,6 +14,11 @@ namespace GUI {
 
 class ModelEditingDelegate {
 public:
+    enum SelectionBehavior {
+        DoNotSelect,
+        SelectAll,
+    };
+
     virtual ~ModelEditingDelegate() { }
 
     void bind(Model& model, const ModelIndex& index)
@@ -32,7 +37,7 @@ public:
     Function<void()> on_rollback;
 
     virtual Variant value() const = 0;
-    virtual void set_value(const Variant&) = 0;
+    virtual void set_value(Variant const&, SelectionBehavior selection_behavior = SelectionBehavior::SelectAll) = 0;
 
     virtual void will_begin_editing() { }
 
@@ -76,11 +81,12 @@ public:
         return textbox;
     }
     virtual Variant value() const override { return static_cast<const TextBox*>(widget())->text(); }
-    virtual void set_value(const Variant& value) override
+    virtual void set_value(Variant const& value, SelectionBehavior selection_behavior) override
     {
         auto& textbox = static_cast<TextBox&>(*widget());
         textbox.set_text(value.to_string());
-        textbox.select_all();
+        if (selection_behavior == SelectionBehavior::SelectAll)
+            textbox.select_all();
     }
 };
 
diff --git a/Userland/Libraries/LibGUI/TableView.cpp b/Userland/Libraries/LibGUI/TableView.cpp
index d26d4a1064..9a904bec49 100644
--- a/Userland/Libraries/LibGUI/TableView.cpp
+++ b/Userland/Libraries/LibGUI/TableView.cpp
@@ -173,14 +173,18 @@ void TableView::keydown_event(KeyEvent& event)
     if (event.is_accepted())
         return;
 
-    auto is_delete = event.key() == Key_Delete || event.key() == Key_Backspace;
-    if (is_editable() && edit_triggers() & EditTrigger::AnyKeyPressed && (event.code_point() != 0 || is_delete)) {
+    auto is_delete = event.key() == Key_Delete;
+    auto is_backspace = event.key() == Key_Backspace;
+    auto is_clear = is_delete || is_backspace;
+    if (is_editable() && edit_triggers() & EditTrigger::AnyKeyPressed && (event.code_point() != 0 || is_clear)) {
         begin_editing(cursor_index());
         if (m_editing_delegate) {
             if (is_delete)
-                m_editing_delegate->set_value(event.key() == Key_Delete ? String {} : String::empty());
+                m_editing_delegate->set_value(String {});
+            else if (is_backspace)
+                m_editing_delegate->set_value(String::empty());
             else
-                m_editing_delegate->set_value(event.text());
+                m_editing_delegate->set_value(event.text(), ModelEditingDelegate::SelectionBehavior::DoNotSelect);
         }
     }
 }