[gnome-contacts/wip/nielsdg/simplify-typecombo] TypeCombo: simplify and document

[Niels De Graef <nielsdg src gnome org>]

commit 824c4c85da0c9153e9124dbf882add9f43c62ab4
Author: Niels De Graef <nielsdegraef gmail com>
Date:   Mon Dec 17 09:47:10 2018 +0100

    TypeCombo: simplify and document
    
    Remove all the code for a custom user-defined label (using an entry).
    This code path has been dead for a while, and it's better to use the
    built-in entry of a ComboBox anyway. This way, we can alo reintroduce it
    later.
    
    We can now also derive directly from a Gtk.ComboBox, which simplifies
    the code even further.
    
    Finally, this commit adds some documentation in the TypeCmbo, which
    hopefully makes it easier to understand for new contributors.

 src/contacts-contact-editor.vala |  16 ++--
 src/contacts-type-combo.vala     | 163 +++++++++++++--------------------------
 2 files changed, 62 insertions(+), 117 deletions(-)
---
diff --git a/src/contacts-contact-editor.vala b/src/contacts-contact-editor.vala
index c641877..dc5ad4a 100644
--- a/src/contacts-contact-editor.vala
+++ b/src/contacts-contact-editor.vala
@@ -200,7 +200,7 @@ public class Contacts.ContactEditor : ContactForm {
       if (entry.get_text () == "")
         continue;
 
-      combo.update_details (row_entry.value.details);
+      combo.active_descriptor.save_to_field_details (row_entry.value.details);
       var details = new EmailFieldDetails (entry.get_text (), row_entry.value.details.parameters);
       new_details.add (details);
     }
@@ -221,7 +221,7 @@ public class Contacts.ContactEditor : ContactForm {
       if (entry.get_text () == "")
         continue;
 
-      combo.update_details (row_entry.value.details);
+      combo.active_descriptor.save_to_field_details (row_entry.value.details);
       var details = new PhoneFieldDetails (entry.get_text (), row_entry.value.details.parameters);
       new_details.add (details);
     }
@@ -306,7 +306,7 @@ public class Contacts.ContactEditor : ContactForm {
     foreach (var row_entry in rows.entries) {
       var combo = container_grid.get_child_at (0, row_entry.key) as TypeCombo;
       var addr_editor = container_grid.get_child_at (1, row_entry.key) as AddressEditor;
-      combo.update_details (row_entry.value.details);
+      combo.active_descriptor.save_to_field_details (row_entry.value.details);
 
       var new_value = new PostalAddress (addr_editor.details.value.po_box,
                                         addr_editor.details.value.extension,
@@ -366,9 +366,9 @@ public class Contacts.ContactEditor : ContactForm {
   void attach_row_with_entry (int row, TypeSet type_set, AbstractFieldDetails details, string value, string? 
type = null) {
     var combo = new TypeCombo (type_set);
     combo.set_hexpand (false);
-    combo.set_active (details);
+    combo.set_active_from_field_details (details);
     if (type != null)
-      combo.set_to (type);
+      combo.set_active_from_vcard_type (type);
     combo.set_valign (Align.CENTER);
     container_grid.attach (combo, 0, row, 1, 1);
 
@@ -388,7 +388,7 @@ public class Contacts.ContactEditor : ContactForm {
     container_grid.attach (delete_button, 3, row, 1, 1);
 
     /* Notify change to upper layer */
-    combo.changed.connect (() => {
+    combo.changed.connect ((c) => {
        set_field_changed (get_current_row (combo));
       });
     value_entry.changed.connect (() => {
@@ -545,9 +545,9 @@ public class Contacts.ContactEditor : ContactForm {
   void attach_row_for_address (int row, TypeSet type_set, PostalAddressFieldDetails details, string? type = 
null) {
     var combo = new TypeCombo (type_set);
     combo.set_hexpand (false);
-    combo.set_active (details);
+    combo.set_active_from_field_details (details);
     if (type != null)
-      combo.set_to (type);
+      combo.set_active_from_vcard_type (type);
     container_grid.attach (combo, 0, row, 1, 1);
 
     var value_address = new AddressEditor (details);
diff --git a/src/contacts-type-combo.vala b/src/contacts-type-combo.vala
index aea3c6e..db998a4 100644
--- a/src/contacts-type-combo.vala
+++ b/src/contacts-type-combo.vala
@@ -19,133 +19,78 @@ using Gtk;
 using Gee;
 using Folks;
 
-public class Contacts.TypeCombo : Grid  {
-  private unowned TypeSet type_set;
-  private ComboBox combo;
-  private Entry entry;
-  private TreeIter last_active;
-  private bool custom_mode;
-  private bool in_manual_change;
-  public bool modified;
-
-  public signal void changed ();
-
-  public TypeCombo (TypeSet type_set) {
-    this.type_set = type_set;
-
-    combo = new ComboBox.with_model (type_set.store);
-    combo.set_halign (Align.FILL);
-    combo.set_hexpand (true);
-    this.add (combo);
-
-    var renderer = new CellRendererText ();
-    combo.pack_start (renderer, true);
-    combo.set_attributes (renderer,
-                          "text", 0);
-    combo.set_row_separator_func ( (model, iter) => {
-        string? s;
-        model.get (iter, 0, out s);
-        return s == null;
-      });
-
-    entry = new Entry ();
-    entry.set_halign (Align.FILL);
-    entry.set_hexpand (true);
-    // Make the default entry small so we don't unnecessarily
-    // expand the labels (it'll be expanded a bit anyway)
-    entry.width_chars = 4;
-
-    this.add (entry);
-
-    combo.set_no_show_all (true);
-    entry.set_no_show_all (true);
-
-    combo.show ();
-
-    combo.changed.connect (combo_changed);
-    entry.focus_out_event.connect (entry_focus_out_event);
-    entry.activate.connect (entry_activate);
-    entry.key_release_event.connect (entry_key_release);
-  }
-
-  private void finish_custom () {
-    if (!custom_mode)
-      return;
+/**
+ * The TypeCombo is a widget that fills itself with the types of a certain
+ * category (using {@link Contacts.TypeSet}). For example, it allows the user
+ * to choose between "Personal", "Home" and "Work" for email addresses,
+ * together with all the custom labels it has encountered since then.
+ */
+public class Contacts.TypeCombo : ComboBox  {
 
-    custom_mode = false;
-    var text = entry.get_text ();
+  private unowned TypeSet type_set;
 
-    if (text != "") {
+  /**
+   * The {@link Contacts.TypeDescriptor} that is currently shown
+   */
+  public TypeDescriptor active_descriptor {
+    get {
       TreeIter iter;
-      type_set.get_iter_for_custom_label (text, out iter);
-
-      last_active = iter;
-      combo.set_active_iter (iter);
-    } else {
-      combo.set_active_iter (last_active);
-    }
-
-    combo.show ();
-    entry.hide ();
-  }
 
-  private void entry_activate () {
-    finish_custom ();
-  }
+      get_active_iter (out iter);
+      assert (!is_separator (this.model, iter));
 
-  private bool entry_key_release (Gdk.EventKey event) {
-    if (event.keyval == Gdk.Key.Escape) {
-      entry.set_text ("");
-      finish_custom ();
+      unowned TypeDescriptor descriptor;
+      this.model.get (iter, 1, out descriptor);
+      return descriptor;
+    }
+    set {
+      set_active_iter (value.iter);
     }
-    return true;
   }
 
-  private bool entry_focus_out_event (Gdk.EventFocus event) {
-    finish_custom ();
-    return false;
-  }
+  construct {
+    this.valign = Align.START;
+    this.halign = Align.FILL;
+    this.hexpand = true;
+    this.visible = true;
 
-  private void combo_changed (ComboBox combo) {
-    if (in_manual_change)
-      return;
+    var renderer = new CellRendererText ();
+    pack_start (renderer, true);
+    set_attributes (renderer, "text", 0);
 
-    modified = true;
-    TreeIter iter;
-    if (combo.get_active_iter (out iter)) {
-      last_active = iter;
-      this.changed ();
-    }
+    set_row_separator_func (is_separator);
   }
 
-  private void set_from_iter (TreeIter iter) {
-    in_manual_change = true;
-    last_active = iter;
-    combo.set_active_iter (iter);
-    in_manual_change = false;
-    modified = false;
+  /**
+   * Creates a TypeCombo for the given TypeSet. To set the active value,
+   * use the "current-decsriptor" property, set_active_from_field_details(),
+   * or set_active_from_vcard_type()
+   */
+  public TypeCombo (TypeSet type_set) {
+    this.type_set = type_set;
+    this.model = type_set.store;
   }
 
-  public void set_active (AbstractFieldDetails details) {
-    TreeIter iter;
-    type_set.get_iter_for_field_details (details, out iter);
-    set_from_iter (iter);
+  private bool is_separator (TreeModel model, TreeIter iter) {
+    unowned string? s;
+    model.get (iter, 0, out s);
+    return s == null;
   }
 
-  public void set_to (string type) {
-    TreeIter iter;
-    type_set.get_iter_for_vcard_type (type, out iter);
-    set_from_iter (iter);
+  /**
+   * Sets the value to the type of the given {@link Folks.AbstractFieldDetails}.
+   */
+  public void set_active_from_field_details (AbstractFieldDetails details) {
+    this.active_descriptor = this.type_set.lookup_descriptor_for_field_details (details);
   }
 
-  public void update_details (AbstractFieldDetails details) {
+  /**
+   * Sets the value to the type that best matches the given vcard type
+   * (for example "HOME" or "WORK").
+   */
+  public void set_active_from_vcard_type (string type) {
     TreeIter iter;
-    combo.get_active_iter (out iter);
-
-    TypeDescriptor descriptor;
-    string display_name;
-    combo.model.get (iter, 0, out display_name, 1, out descriptor);
-    assert (display_name != null); // Not separator
-    descriptor.save_to_field_details (details);
+    this.type_set.get_iter_for_vcard_type (type, out iter);
+    set_active_iter (iter);
   }
 }