Merge lp:~hegarty-sam/midori/midori-inline-speeddialtitle into lp:midori

[Sam Hegarty]

Inline renaming of speed dials

Change how the user edits the title of shortcuts on the speed dial so instead of a prompt displaying it's done inline.

[Sam Hegarty (hegarty-sam)]

review: Approve

[Danielle Foré (danrabbit)]

Is there a reason for changing the indentation on diffline 9?

needs a space after the : on diffline 17

Maybe use "transparent" instead of "rgba(0, 0, 0, 0)"

I can confirm that this fixes the issue :)

review: Needs Fixing

[Sam Hegarty (hegarty-sam)]

Woops, didn't notice I'd changed the indentation because of my editor settings. Fixed the other bits as well. Thanks for taking a look!

[Paweł Forysiuk (tuxator)]

Please don't set review status yourself. Reviewer will do it for you once review is done.

Maybe i am missing something obvious, but there seems to be no way now to cancel the edit.
If you fat-finger or delete something you did not want to you seem to stuck with it now.
Also you may not remember what was there before the edit.
Seems like whatever was in the edit entry you are stuck with now.

First thing i would try as a user to cancel the edit would be hitting esc or clicking somewhere away from the dial tile (maybe highlight subtly where the given tile ends in the title edit mode?)

Other than that it looks mostly fine from the quick glance.

review: Needs Fixing

[Sam Hegarty (hegarty-sam)]

Ah good point. ctrl+z can be used to undo but it would be nice to have a proper way to cancel. What about an 'x' inside the text-box to click?

[Danielle Foré (danrabbit)]

I don't think there really needs to be a graphical way to cancel the change as long as there is an undo. There's no cancel when you're renaming in the file browser and I've never heard of this being a major issue people have with file browsers.

Adding 'esc' to cancel would probably be fine. I would personally expect the box losing focus (by clicking outside it for example) to save the change, not to cancel it.

[Sam Hegarty (hegarty-sam)]

Alright, I've made pressing escape reset the input to what it was when they started changing it.

[Sam Hegarty (hegarty-sam)]

review: Needs Resubmitting

[Sam Hegarty (hegarty-sam)]

Realized after commiting I'd read your comment wrong. Woops.

[Cris Dywan (kalikiana)]

I love it. Unfortunately I completely forgot to actually approve it, sorry :-D It works very well for me in testing. The only "drawback" here is, that it sort of raises the bar of the speed dial UX as far as adding via the dialog and lack of undo go ;-)

Would be thrilled to talk about follow up work!

review: Approve

=== modified file 'data/speeddial-head.html'
--- data/speeddial-head.html	2013-06-25 16:12:25 +0000
+++ data/speeddial-head.html	2014-11-15 09:02:34 +0000
@@ -27,7 +27,7 @@
         cursor: default;
         font-size: 13px;
         color: #4d4d4d;
-	-webkit-user-select: none;
+        -webkit-user-select: none;
     }
 
     html, body {
@@ -85,8 +85,13 @@
     }
 
     .title {
+        background: transparent;
+        border: 1px solid transparent;
+        display: block;
         text-align: center;
         margin: 8px;
+        margin-left: auto;
+        margin-right: auto;
         white-space: nowrap;
         text-overflow: ellipsis;
         overflow: hidden;
@@ -119,9 +124,24 @@
         outline: 1px dotted black;
         background-color: #eef;
     }
+
+    .selectable {
+        -webkit-user-select: text;
+    }
     </style>
 
     <script type="text/javascript">
+    var previousName = "";
+
+    function input_key_down (ev) {
+        // 13 is the key code for enter
+        if(ev.keyCode == 13) ev.target.blur();
+        // 27 is the key code for escape
+        if(ev.keyCode == 27) {
+            ev.target.value = previousName;
+            ev.target.blur();
+        }
+    }
 
     function add_tile (ev) {
         ev.preventDefault();
@@ -137,17 +157,26 @@
         console.log ("speed_dial-save-add " + id + " " + url);
     }
 
-    function rename_tile (ev) {
-        var old_name = ev.target.textContent;
-
-        var name = prompt ("{enter_shortcut_name}", old_name);
-        if (!name)
-            return;
-
+    function done_editing_title (ev) {
+        input = ev.target;
+        input.style.backgroundColor = "transparent";
+        input.style.border = "1px solid transparent";
+
+        var name = ev.target.value;
+        if (name == "")
+            name = previousName;
         var id = ev.target.parentNode.id;
         console.log ("speed_dial-save-rename " + id + " " + name);
     }
 
+    function editing_tile (ev) {
+        input = ev.target;
+        previousName = input.value;
+        // indicate to user they are editing the title
+        input.style.backgroundColor = "#F8F8F8";
+        input.style.border = "1px solid #808060";
+    }
+
     function delete_tile (ev) {
         ev.preventDefault();
 
@@ -158,7 +187,6 @@
         console.log ("speed_dial-save-delete " + id);
     }
 
-
     var firstNode, secondNode;
     var cursor;
 
@@ -177,6 +205,23 @@
         if (ev == undefined)
             return;
 
+        /* If the target of the event is an element of class title
+         * and not also of class add-shortcut we are editing title
+         */
+        var classes = ev.target.className.split(" ");
+        var edit_title = false;
+        for (var i = 0; i < classes.length; i++) {
+            if(classes[i] == "add-shortcut") {
+                edit_title = false;
+                break;
+            }
+            if(classes[i] == "title") 
+                edit_title = true;
+        }
+
+        if(edit_title) 
+            return;
+
         ev.preventDefault();
         var ele = ev.target;
         cursor = ele.style.cursor;
@@ -198,7 +243,8 @@
         var eparent = get_dial_div (ele);
 
         ele.style.cursor = cursor;
-        secondNode = eparent.id;
+        if(eparent != undefined) secondNode = eparent.id;
+        else return;
 
         /* ommit just mere clicking the dial */
         if (firstNode != secondNode && firstNode != undefined)
@@ -237,8 +283,11 @@
         var titles = document.getElementsByClassName ("title");
         var len = titles.length;
         for (var i = 0; i < len; i++) {
-            if (titles[i].parentNode.childNodes[0].className != "preview new")
-                titles[i].addEventListener ('click', rename_tile, false);
+            if (titles[i].parentNode.childNodes[0].className != "preview new") {
+                titles[i].addEventListener ('click', editing_tile, false);
+                titles[i].addEventListener ('blur', done_editing_title, false);
+                titles[i].addEventListener ('keydown', input_key_down, false);
+            }
         }
 
         var crosses = document.getElementsByClassName ("cross");
 
=== modified file 'midori/midori-speeddial.vala'
--- midori/midori-speeddial.vala	2013-10-28 23:08:02 +0000
+++ midori/midori-speeddial.vala	2014-11-15 09:02:34 +0000
@@ -188,7 +188,6 @@
                 string header = head.replace ("{title}", _("Speed Dial")).
                     replace ("{click_to_add}", _("Click to add a shortcut")).
                     replace ("{enter_shortcut_address}", _("Enter shortcut address")).
-                    replace ("{enter_shortcut_name}", _("Enter shortcut title")).
                     replace ("{are_you_sure}", _("Are you sure you want to delete this shortcut?"));
                 var markup = new StringBuilder (header);
 
@@ -251,7 +250,7 @@
                                 <div class="shortcut" id="%u"><div class="preview">
                                 <a class="cross" href="#"></a>
                                 <a href="%s"><img src="data:image/png;base64,%s" title='%s'></a>
-                                </div><div class="title">%s</div></div>
+                                </div><input type="text" class="title selectable" value="%s"></div>
                                 """,
                                 slot, uri, encoded ?? "", title, title ?? "");
                         }