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

Proposed by Sam Hegarty
Status: Merged
Approved by: Cris Dywan
Approved revision: 6823
Merged at revision: 6839
Proposed branch: lp:~hegarty-sam/midori/midori-inline-speeddialtitle
Merge into: lp:midori
Diff against target: 162 lines (+62/-14)
2 files modified
data/speeddial-head.html (+61/-12)
midori/midori-speeddial.vala (+1/-2)
To merge this branch: bzr merge lp:~hegarty-sam/midori/midori-inline-speeddialtitle
Reviewer Review Type Date Requested Status
Cris Dywan Approve
Sam Hegarty Pending
Paweł Forysiuk Pending
Danielle Foré Pending
Review via email: mp+241171@code.launchpad.net

This proposal supersedes a proposal from 2014-10-06.

Commit message

Inline renaming of speed dials

Description of the change

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

To post a comment you must log in.
Revision history for this message
Sam Hegarty (hegarty-sam) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
Danielle Foré (danrabbit) wrote : Posted in a previous version of this proposal

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
Revision history for this message
Sam Hegarty (hegarty-sam) wrote : Posted in a previous version of this proposal

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!

Revision history for this message
Paweł Forysiuk (tuxator) wrote : Posted in a previous version of this proposal

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
Revision history for this message
Sam Hegarty (hegarty-sam) wrote : Posted in a previous version of this proposal

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?

Revision history for this message
Danielle Foré (danrabbit) wrote : Posted in a previous version of this proposal

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.

Revision history for this message
Sam Hegarty (hegarty-sam) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Sam Hegarty (hegarty-sam) : Posted in a previous version of this proposal
review: Needs Resubmitting
Revision history for this message
Sam Hegarty (hegarty-sam) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Cris Dywan (kalikiana) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'data/speeddial-head.html'
2--- data/speeddial-head.html 2013-06-25 16:12:25 +0000
3+++ data/speeddial-head.html 2014-11-15 09:02:34 +0000
4@@ -27,7 +27,7 @@
5 cursor: default;
6 font-size: 13px;
7 color: #4d4d4d;
8- -webkit-user-select: none;
9+ -webkit-user-select: none;
10 }
11
12 html, body {
13@@ -85,8 +85,13 @@
14 }
15
16 .title {
17+ background: transparent;
18+ border: 1px solid transparent;
19+ display: block;
20 text-align: center;
21 margin: 8px;
22+ margin-left: auto;
23+ margin-right: auto;
24 white-space: nowrap;
25 text-overflow: ellipsis;
26 overflow: hidden;
27@@ -119,9 +124,24 @@
28 outline: 1px dotted black;
29 background-color: #eef;
30 }
31+
32+ .selectable {
33+ -webkit-user-select: text;
34+ }
35 </style>
36
37 <script type="text/javascript">
38+ var previousName = "";
39+
40+ function input_key_down (ev) {
41+ // 13 is the key code for enter
42+ if(ev.keyCode == 13) ev.target.blur();
43+ // 27 is the key code for escape
44+ if(ev.keyCode == 27) {
45+ ev.target.value = previousName;
46+ ev.target.blur();
47+ }
48+ }
49
50 function add_tile (ev) {
51 ev.preventDefault();
52@@ -137,17 +157,26 @@
53 console.log ("speed_dial-save-add " + id + " " + url);
54 }
55
56- function rename_tile (ev) {
57- var old_name = ev.target.textContent;
58-
59- var name = prompt ("{enter_shortcut_name}", old_name);
60- if (!name)
61- return;
62-
63+ function done_editing_title (ev) {
64+ input = ev.target;
65+ input.style.backgroundColor = "transparent";
66+ input.style.border = "1px solid transparent";
67+
68+ var name = ev.target.value;
69+ if (name == "")
70+ name = previousName;
71 var id = ev.target.parentNode.id;
72 console.log ("speed_dial-save-rename " + id + " " + name);
73 }
74
75+ function editing_tile (ev) {
76+ input = ev.target;
77+ previousName = input.value;
78+ // indicate to user they are editing the title
79+ input.style.backgroundColor = "#F8F8F8";
80+ input.style.border = "1px solid #808060";
81+ }
82+
83 function delete_tile (ev) {
84 ev.preventDefault();
85
86@@ -158,7 +187,6 @@
87 console.log ("speed_dial-save-delete " + id);
88 }
89
90-
91 var firstNode, secondNode;
92 var cursor;
93
94@@ -177,6 +205,23 @@
95 if (ev == undefined)
96 return;
97
98+ /* If the target of the event is an element of class title
99+ * and not also of class add-shortcut we are editing title
100+ */
101+ var classes = ev.target.className.split(" ");
102+ var edit_title = false;
103+ for (var i = 0; i < classes.length; i++) {
104+ if(classes[i] == "add-shortcut") {
105+ edit_title = false;
106+ break;
107+ }
108+ if(classes[i] == "title")
109+ edit_title = true;
110+ }
111+
112+ if(edit_title)
113+ return;
114+
115 ev.preventDefault();
116 var ele = ev.target;
117 cursor = ele.style.cursor;
118@@ -198,7 +243,8 @@
119 var eparent = get_dial_div (ele);
120
121 ele.style.cursor = cursor;
122- secondNode = eparent.id;
123+ if(eparent != undefined) secondNode = eparent.id;
124+ else return;
125
126 /* ommit just mere clicking the dial */
127 if (firstNode != secondNode && firstNode != undefined)
128@@ -237,8 +283,11 @@
129 var titles = document.getElementsByClassName ("title");
130 var len = titles.length;
131 for (var i = 0; i < len; i++) {
132- if (titles[i].parentNode.childNodes[0].className != "preview new")
133- titles[i].addEventListener ('click', rename_tile, false);
134+ if (titles[i].parentNode.childNodes[0].className != "preview new") {
135+ titles[i].addEventListener ('click', editing_tile, false);
136+ titles[i].addEventListener ('blur', done_editing_title, false);
137+ titles[i].addEventListener ('keydown', input_key_down, false);
138+ }
139 }
140
141 var crosses = document.getElementsByClassName ("cross");
142
143=== modified file 'midori/midori-speeddial.vala'
144--- midori/midori-speeddial.vala 2013-10-28 23:08:02 +0000
145+++ midori/midori-speeddial.vala 2014-11-15 09:02:34 +0000
146@@ -188,7 +188,6 @@
147 string header = head.replace ("{title}", _("Speed Dial")).
148 replace ("{click_to_add}", _("Click to add a shortcut")).
149 replace ("{enter_shortcut_address}", _("Enter shortcut address")).
150- replace ("{enter_shortcut_name}", _("Enter shortcut title")).
151 replace ("{are_you_sure}", _("Are you sure you want to delete this shortcut?"));
152 var markup = new StringBuilder (header);
153
154@@ -251,7 +250,7 @@
155 <div class="shortcut" id="%u"><div class="preview">
156 <a class="cross" href="#"></a>
157 <a href="%s"><img src="data:image/png;base64,%s" title='%s'></a>
158- </div><div class="title">%s</div></div>
159+ </div><input type="text" class="title selectable" value="%s"></div>
160 """,
161 slot, uri, encoded ?? "", title, title ?? "");
162 }

Subscribers

People subscribed via source and target branches

to all changes: