[geary: 1/2] Clean up composer link insertion and deletion



commit 52d010bb224fb76c9e6909b0edc0b9b35d7e6193
Author: Alex Henrie <alexhenrie24 gmail com>
Date:   Sat Oct 5 10:25:25 2019 -0600

    Clean up composer link insertion and deletion
    
    Allow deleting only the selected part of a link, and modify the existing
    link instead of inserting a new one if the user didn't select any text.
    Also restructure the code to avoid always-true or always-false if
    statements.
    
    Fixes #591

 src/client/composer/composer-web-view.vala | 13 ++++++++---
 src/client/composer/composer-widget.vala   |  2 +-
 ui/composer-web-view.js                    | 35 +++++++++++++++---------------
 3 files changed, 29 insertions(+), 21 deletions(-)
---
diff --git a/src/client/composer/composer-web-view.vala b/src/client/composer/composer-web-view.vala
index 0f230f7c..ee495a5b 100644
--- a/src/client/composer/composer-web-view.vala
+++ b/src/client/composer/composer-web-view.vala
@@ -344,10 +344,17 @@ public class ComposerWebView : ClientWebView {
     }
 
     /**
-     * Removes any A element at the current text cursor location.
+     * Removes the A element at the current text cursor location.
+     *
+     * If only part of the A element is selected, only that part is
+     * unlinked, possibly creating two new A elements flanking the
+     * unlinked section.
      */
-    public void delete_link() {
-        this.call.begin(Util.JS.callable("geary.deleteLink"), null);
+    public void delete_link(string selection_id) {
+        this.call.begin(
+            Util.JS.callable("geary.deleteLink").string(selection_id),
+            null
+        );
     }
 
     /**
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index 3cbc21b6..31a0dff8 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -2330,7 +2330,7 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface {
                 this.editor.insert_link(popover.link_uri, selection_id);
             });
         popover.link_delete.connect(() => {
-                this.editor.delete_link();
+                this.editor.delete_link(selection_id);
             });
         popover.link_open.connect(() => { link_activated(popover.link_uri); });
         return popover;
diff --git a/ui/composer-web-view.js b/ui/composer-web-view.js
index 524850ca..a848bb1a 100644
--- a/ui/composer-web-view.js
+++ b/ui/composer-web-view.js
@@ -163,30 +163,31 @@ ComposerPageState.prototype = {
         this.selections.delete(id);
     },
     insertLink: function(href, selectionId) {
-        if (!window.getSelection().isCollapsed) {
-            // There is currently a selection, so assume the user
-            // knows what they are doing and just linkify it.
+        SelectionUtil.restore(this.selections.get(selectionId));
+        if (window.getSelection().isCollapsed) {
+            // The saved selection was empty, which means that the user is
+            // modifying an existing link instead of inserting a new one.
+            let selection = SelectionUtil.save();
+            let selected = SelectionUtil.getCursorElement();
+            SelectionUtil.selectNode(selected);
             document.execCommand("createLink", false, href);
+            SelectionUtil.restore(selection);
         } else {
-            SelectionUtil.restore(this.selections.get(selectionId));
             document.execCommand("createLink", false, href);
         }
     },
-    deleteLink: function() {
-        if (!window.getSelection().isCollapsed) {
-            // There is currently a selection, so assume the user
-            // knows what they are doing and just unlink it.
+    deleteLink: function(selectionId) {
+        SelectionUtil.restore(this.selections.get(selectionId));
+        if (window.getSelection().isCollapsed) {
+            // The saved selection was empty, which means that the user is
+            // deleting the entire existing link.
+            let selection = SelectionUtil.save();
+            let selected = SelectionUtil.getCursorElement();
+            SelectionUtil.selectNode(selected);
             document.execCommand("unlink", false, null);
+            SelectionUtil.restore(selection);
         } else {
-            let selected = SelectionUtil.getCursorElement();
-            if (selected != null && selected.tagName == "A") {
-                // The current cursor element is an A, so select it
-                // since unlink requires a range
-                let selection = SelectionUtil.save();
-                SelectionUtil.selectNode(selected);
-                document.execCommand("unlink", false, null);
-                SelectionUtil.restore(selection);
-            }
+            document.execCommand("unlink", false, null);
         }
     },
     indentLine: function() {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]