[geary: 4/6] composer: Improve "insert link" popover

[Michael Gratton <mjog src gnome org>]

commit adf6f2b4a7529397d23f1033a0ceaaf56237936a
Author: James Westman <james flyingpimonster net>
Date:   Thu Jan 30 14:17:41 2020 -0600

    composer: Improve "insert link" popover
    
    - Use transitions instead of show/hide
    - Make the popover narrower (360px instead of 40 chars)
    - Use text labels for "Add" and "Insert" rather than the check icon
    - Show the button as active when the popover is open

 src/client/composer/composer-link-popover.vala | 22 ++-----------
 src/client/composer/composer-widget.vala       | 13 +++++---
 ui/composer-link-popover.ui                    | 43 ++++----------------------
 3 files changed, 17 insertions(+), 61 deletions(-)
---
diff --git a/src/client/composer/composer-link-popover.vala b/src/client/composer/composer-link-popover.vala
index 048c50f9..a4774a40 100644
--- a/src/client/composer/composer-link-popover.vala
+++ b/src/client/composer/composer-link-popover.vala
@@ -50,10 +50,7 @@ public class Composer.LinkPopover : Gtk.Popover {
     private Gtk.Button update;
 
     [GtkChild]
-    private Gtk.Button delete;
-
-    [GtkChild]
-    private Gtk.Button open;
+    private Gtk.Button remove;
 
     private Geary.TimeoutManager validation_timeout;
 
@@ -64,9 +61,6 @@ public class Composer.LinkPopover : Gtk.Popover {
     /** Emitted when the link URL was activated. */
     public signal void link_activate();
 
-    /** Emitted when the open button was activated. */
-    public signal void link_open();
-
     /** Emitted when the delete button was activated. */
     public signal void link_delete();
 
@@ -77,7 +71,7 @@ public class Composer.LinkPopover : Gtk.Popover {
         switch (type) {
         case Type.NEW_LINK:
             this.update.hide();
-            this.delete.hide();
+            this.remove.hide();
             break;
         case Type.EXISTING_LINK:
             this.insert.hide();
@@ -135,10 +129,6 @@ public class Composer.LinkPopover : Gtk.Popover {
             }
         }
 
-        // Don't let the user open invalid and mailto links, it's not
-        // terribly useful
-        this.open.set_sensitive(is_nominal && !is_mailto);
-
         Gtk.StyleContext style = this.url.get_style_context();
         Gtk.EntryIconPosition pos = Gtk.EntryIconPosition.SECONDARY;
         if (!is_valid) {
@@ -177,14 +167,8 @@ public class Composer.LinkPopover : Gtk.Popover {
     }
 
     [GtkCallback]
-    private void on_delete_clicked() {
+    private void on_remove_clicked() {
         link_delete();
         popdown();
     }
-
-    [GtkCallback]
-    private void on_open_clicked() {
-        link_open();
-    }
-
 }
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index ac248221..ebc4a0d6 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -2640,6 +2640,8 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
         popover.set_link_url(url);
         popover.closed.connect(() => {
                 this.editor.free_selection(selection_id);
+            });
+        popover.hide.connect(() => {
                 Idle.add(() => { popover.destroy(); return Source.REMOVE; });
             });
         popover.link_activate.connect((link_uri) => {
@@ -2648,9 +2650,6 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
         popover.link_delete.connect(() => {
                 this.editor.delete_link(selection_id);
             });
-        popover.link_open.connect(() => {
-                this.application.show_uri.begin(popover.link_uri);
-            });
         return popover;
     }
 
@@ -2721,7 +2720,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
                     LinkPopover popover = this.new_link_popover.end(res);
                     popover.set_relative_to(this.editor);
                     popover.set_pointing_to(location);
-                    popover.show();
+                    popover.popup();
                 });
         }
         return Gdk.EVENT_PROPAGATE;
@@ -2833,6 +2832,8 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
         this.new_link_popover.begin(type, url, (obj, res) => {
                 LinkPopover popover = this.new_link_popover.end(res);
 
+                var style = this.insert_link_button.get_style_context();
+
                 // We have to disconnect then reconnect the selection
                 // changed signal for the duration of the popover
                 // being active since if the user selects the text in
@@ -2842,10 +2843,12 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
                 this.editor.selection_changed.disconnect(on_selection_changed);
                 popover.closed.connect(() => {
                         this.editor.selection_changed.connect(on_selection_changed);
+                        style.set_state(NORMAL);
                     });
 
                 popover.set_relative_to(this.insert_link_button);
-                popover.show();
+                popover.popup();
+                style.set_state(ACTIVE);
             });
     }
 
diff --git a/ui/composer-link-popover.ui b/ui/composer-link-popover.ui
index 0d4a9396..463578ed 100644
--- a/ui/composer-link-popover.ui
+++ b/ui/composer-link-popover.ui
@@ -5,6 +5,7 @@
   <template class="ComposerLinkPopover" parent="GtkPopover">
     <property name="can_focus">False</property>
     <property name="position">top</property>
+    <property name="width-request">360</property>
     <child>
       <object class="GtkGrid">
         <property name="visible">True</property>
@@ -20,7 +21,7 @@
             <property name="visible">True</property>
             <property name="can_focus">True</property>
             <property name="can_default">True</property>
-            <property name="width_chars">40</property>
+            <property name="hexpand">True</property>
             <property name="primary_icon_activatable">False</property>
             <property name="secondary_icon_activatable">False</property>
             <property name="placeholder_text">https://</property>
@@ -39,14 +40,8 @@
             <property name="can_focus">True</property>
             <property name="receives_default">True</property>
             <property name="tooltip_text" translatable="yes" comments="Note that this button and the Update 
button will never be shown at the same time to the user.">Insert the new link with this URL</property>
+            <property name="label" translatable="yes">Add</property>
             <signal name="clicked" handler="on_activate_popover" swapped="no"/>
-            <child>
-              <object class="GtkImage">
-                <property name="visible">True</property>
-                <property name="can_focus">False</property>
-                <property name="icon_name">emblem-ok-symbolic</property>
-              </object>
-            </child>
             <child internal-child="accessible">
               <object class="AtkObject" id="insert-atkobject">
                 <property name="AtkObject::accessible-name" translatable="yes">Link URL</property>
@@ -64,14 +59,8 @@
             <property name="can_focus">True</property>
             <property name="receives_default">True</property>
             <property name="tooltip_text" translatable="yes" comments="Note that this button and the Insert 
button will never be shown at the same time to the user.">Update this link’s URL</property>
+            <property name="label" translatable="yes">Update</property>
             <signal name="clicked" handler="on_activate_popover" swapped="no"/>
-            <child>
-              <object class="GtkImage">
-                <property name="visible">True</property>
-                <property name="can_focus">False</property>
-                <property name="icon_name">emblem-ok-symbolic</property>
-              </object>
-            </child>
           </object>
           <packing>
             <property name="left_attach">2</property>
@@ -79,12 +68,12 @@
           </packing>
         </child>
         <child>
-          <object class="GtkButton" id="delete">
+          <object class="GtkButton" id="remove">
             <property name="visible">True</property>
             <property name="can_focus">True</property>
             <property name="receives_default">True</property>
             <property name="tooltip_text" translatable="yes">Delete this link</property>
-            <signal name="clicked" handler="on_delete_clicked" swapped="no"/>
+            <signal name="clicked" handler="on_remove_clicked" swapped="no"/>
             <child>
               <object class="GtkImage">
                 <property name="visible">True</property>
@@ -98,26 +87,6 @@
             <property name="top_attach">0</property>
           </packing>
         </child>
-        <child>
-          <object class="GtkButton" id="open">
-            <property name="visible">True</property>
-            <property name="can_focus">True</property>
-            <property name="receives_default">True</property>
-            <property name="tooltip_text" translatable="yes">Open this link</property>
-            <signal name="clicked" handler="on_open_clicked" swapped="no"/>
-            <child>
-              <object class="GtkImage">
-                <property name="visible">True</property>
-                <property name="can_focus">False</property>
-                <property name="icon_name">document-open-symbolic</property>
-              </object>
-            </child>
-          </object>
-          <packing>
-            <property name="left_attach">4</property>
-            <property name="top_attach">0</property>
-          </packing>
-        </child>
       </object>
     </child>
   </template>