Merge lp:~jeremywootten/pantheon-files/fix-sidebar-dragging into lp:~elementary-apps/pantheon-files/trunk

Proposed by Jeremy Wootten
Status: Merged
Approved by: Cody Garver
Approved revision: 1884
Merged at revision: 1885
Proposed branch: lp:~jeremywootten/pantheon-files/fix-sidebar-dragging
Merge into: lp:~elementary-apps/pantheon-files/trunk
Diff against target: 291 lines (+115/-56)
1 file modified
src/View/Sidebar.vala (+115/-56)
To merge this branch: bzr merge lp:~jeremywootten/pantheon-files/fix-sidebar-dragging
Reviewer Review Type Date Requested Status
Fabio Zaramella (community) Approve
Cody Garver Pending
Review via email: mp+265188@code.launchpad.net

Commit message

Dragging a bookmark off the sidebar now properly removes it (lp:1473754)

Disallow dragging bookmarks that cannot be removed (lp:1474410)

Description of the change

This branch fixes issues related to the dragging and dropping of bookmarks caused by the selected bookmark sometimes changing after drag started:

Dragging off the sidebar did not destroy the bookmark unless it was current view
Dragging an unselected bookmark could have unexpected results.

Also dragging is inhibited for bookmarks that are not intended to be moved.

To post a comment you must log in.
Revision history for this message
Fabio Zaramella (fabiozaramella) wrote :

Works fine.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/View/Sidebar.vala'
2--- src/View/Sidebar.vala 2015-07-12 16:21:55 +0000
3+++ src/View/Sidebar.vala 2015-07-18 08:54:19 +0000
4@@ -51,6 +51,8 @@
5
6 /* DnD */
7 List<GLib.File> drag_list;
8+ Gtk.TreeRowReference? drag_row_ref;
9+ bool dnd_disabled = false;
10 uint drag_data_info;
11 uint drag_scroll_timer_id;
12 Gdk.DragContext drag_context;
13@@ -105,6 +107,9 @@
14 /* Remember vertical adjustment value when lose focus */
15 double adjustment_val = 0.0;
16
17+ /* Remember path at button press */
18+ Gtk.TreePath? click_path = null;
19+
20 public signal void sync_needed ();
21
22 public Sidebar (Marlin.View.Window window) {
23@@ -724,7 +729,11 @@
24 device.get_position (null, out x, out y);
25 poof_window = Plank.Widgets.PoofWindow.get_default ();
26 poof_window.show_at (x, y);
27- remove_selected_bookmarks ();
28+ if (drag_row_ref != null) {
29+ Gtk.TreeIter iter;
30+ store.get_iter (out iter, drag_row_ref.get_path ());
31+ remove_bookmark_iter (iter);
32+ }
33 return true;
34 } else
35 return false;
36@@ -820,13 +829,19 @@
37 uint info,
38 uint time) {
39 if (!received_drag_data) {
40+ this.drag_list = null;
41+ this.drag_row_ref = null;
42 if (selection_data.get_target () != Gdk.Atom.NONE
43 && info == TargetType.TEXT_URI_LIST) {
44 string s = (string)(selection_data.get_data ());
45 drag_list = EelGFile.list_new_from_string (s);
46- } else
47- this.drag_list = null;
48-
49+ } else {
50+ if (info == TargetType.GTK_TREE_MODEL_ROW) {
51+ Gtk.TreePath path;
52+ Gtk.tree_get_row_drag_data (selection_data, null, out path);
53+ drag_row_ref = new Gtk.TreeRowReference (store, path);
54+ }
55+ }
56 received_drag_data = true;
57 drag_data_info = info;
58 }
59@@ -1151,10 +1166,8 @@
60 if (!get_selected_iter ( out iter))
61 return;
62
63- bool is_bookmark;
64- store.@get (iter, Column.BOOKMARK, out is_bookmark, -1);
65- if (!is_bookmark)
66- return;
67+ if (!bookmark_at_iter (iter))
68+ return;
69
70 var path = store.get_path (iter);
71 var column = tree_view.get_column (0);
72@@ -1172,11 +1185,16 @@
73 if (!get_selected_iter (out iter))
74 return;
75
76- bool is_bookmark;
77- store.@get (iter, Column.BOOKMARK, out is_bookmark, -1);
78- if (!is_bookmark)
79+ remove_bookmark_iter (iter);
80+ }
81+
82+ private void remove_bookmark_iter (Gtk.TreeIter? iter) {
83+ if (iter == null)
84 return;
85
86+ if (!bookmark_at_iter (iter))
87+ return;
88+
89 uint index;
90 store.@get (iter, Column.INDEX, out index);
91 index = index <= n_builtins_before ? 0 : index - n_builtins_before;
92@@ -1185,26 +1203,27 @@
93
94 /* Reorder the selected bookmark to the specified position */
95 private void reorder_bookmarks (uint new_position) {
96- /* Get the selected path */
97- Gtk.TreeIter iter;
98- if (!get_selected_iter (out iter))
99- return;
100-
101- bool is_bookmark;
102- uint old_position;
103- store.@get (iter,
104- Column.BOOKMARK, out is_bookmark,
105- Column.INDEX, out old_position);
106-
107- if (old_position <= n_builtins_before)
108- old_position = 0;
109- else
110- old_position-= n_builtins_before;
111-
112- if (!is_bookmark || old_position >= bookmarks.length ())
113- return;
114-
115- bookmarks.move_item (old_position, new_position);
116+ if (drag_row_ref != null) {
117+ Gtk.TreeIter iter;
118+ store.get_iter (out iter, drag_row_ref.get_path ());
119+ drag_row_ref = null;
120+
121+ if (!bookmark_at_iter (iter))
122+ return;
123+
124+ uint old_position;
125+ store.@get (iter, Column.INDEX, out old_position);
126+
127+ if (old_position <= n_builtins_before)
128+ old_position = 0;
129+ else
130+ old_position-= n_builtins_before;
131+
132+ if (old_position >= bookmarks.length ())
133+ return;
134+
135+ bookmarks.move_item (old_position, new_position);
136+ }
137 }
138
139 /* POPUP MENU FUNCTIONS */
140@@ -1530,9 +1549,7 @@
141 }
142
143 private bool button_press_event_cb (Gtk.Widget widget, Gdk.EventButton event) {
144- if (event.type != Gdk.EventType.BUTTON_PRESS)
145- return true;
146-
147+ click_path = null;
148 var tree_view = widget as Gtk.TreeView;
149 if (event.window != tree_view.get_bin_window ())
150 return true;
151@@ -1540,14 +1557,13 @@
152 if (renaming)
153 return true;
154
155- int tx, ty;
156- tree_view.convert_bin_window_to_tree_coords ((int)event.x, (int)event.y, out tx, out ty);
157- Gtk.TreePath? path = null;
158- tree_view.get_path_at_pos (tx, ty, out path, null, null, null);
159-
160+ Gtk.TreePath? path = get_path_at_click_position (event);
161 if (path == null)
162 return false;
163
164+ this.click_path = path.copy ();
165+
166+
167 switch (event.button) {
168 case Gdk.BUTTON_PRIMARY:
169 /* If the user clicked over a category, toggle expansion. The entire row
170@@ -1560,7 +1576,9 @@
171 tree_view.expand_row (path, false);
172
173 return true;
174- }
175+ } else if (!bookmark_at_path (path))
176+ block_drag_and_drop ();
177+
178 break;
179
180 case Gdk.BUTTON_SECONDARY:
181@@ -1580,17 +1598,21 @@
182 }
183
184 private bool button_release_event_cb (Gtk.Widget widget, Gdk.EventButton event) {
185- if (event.type != Gdk.EventType.BUTTON_RELEASE)
186- return true;
187-
188- if (renaming)
189- return true;
190-
191- int tx, ty;
192- tree_view.convert_bin_window_to_tree_coords ((int)event.x, (int)event.y, out tx, out ty);
193-
194- Gtk.TreePath? path = null;
195- if (over_eject_button (tx, ty, out path)) { /* returns path whether or not over eject button */
196+ Gtk.TreePath? path = get_path_at_click_position (event);
197+
198+ if (dnd_disabled) {
199+ unblock_drag_and_drop ();
200+ /* Do not take action if a blocked drag was attempted (mouse over different row
201+ * from when button pressed or not over row) */
202+ if (path == null || (click_path != null && path.compare (click_path) != 0)) {
203+ return true;
204+ }
205+ }
206+
207+ if (renaming)
208+ return true;
209+
210+ if (over_eject_button (event)) {
211 eject_or_unmount_bookmark (path);
212 return false;
213 }
214@@ -1599,8 +1621,6 @@
215 if (event.window != tree_view.get_bin_window ())
216 return false;
217
218- tree_view.get_path_at_pos ((int)(event.x), (int)(event.y), out path, null, null, null);
219-
220 if (path != null) {
221 if ((event.state & Gdk.ModifierType.CONTROL_MASK) != 0)
222 open_selected_bookmark (store, path, Marlin.OpenFlag.NEW_TAB);
223@@ -1649,20 +1669,21 @@
224 }
225 }
226
227- private bool over_eject_button (int x, int y, out Gtk.TreePath p) {
228+ private bool over_eject_button (Gdk.EventButton event) {
229 unowned Gtk.TreeViewColumn column;
230 int width, x_offset, hseparator;
231 bool show_eject;
232 Gtk.TreeIter iter;
233 Gtk.TreePath path;
234
235- p = null;
236+ int x, y;
237+ tree_view.convert_bin_window_to_tree_coords ((int)event.x, (int)event.y, out x, out y);
238+
239 int cell_x, cell_y;
240 if (tree_view.get_path_at_pos (x, y, out path, out column, out cell_x, out cell_y)) {
241 if (path == null)
242 return false;
243
244- p = path; /* Return path either way */
245 store.get_iter (out iter, path);
246 store.@get (iter, Column.EJECT, out show_eject);
247
248@@ -2073,5 +2094,43 @@
249 return path.get_depth () == 1;
250 }
251
252+ private bool bookmark_at_path (Gtk.TreePath? path) {
253+ if (path != null) {
254+ Gtk.TreeIter iter;
255+ store.get_iter (out iter, path);
256+ return bookmark_at_iter (iter);
257+ } else
258+ return false;
259+ }
260+
261+ private bool bookmark_at_iter (Gtk.TreeIter? iter) {
262+ if (iter == null)
263+ return false;
264+
265+ bool is_bookmark;
266+ store.@get (iter, Column.BOOKMARK, out is_bookmark, -1);
267+ return is_bookmark;
268+ }
269+
270+ private Gtk.TreePath? get_path_at_click_position (Gdk.EventButton event) {
271+ int tx, ty;
272+ tree_view.convert_bin_window_to_tree_coords ((int)event.x, (int)event.y, out tx, out ty);
273+ Gtk.TreePath? path = null;
274+ tree_view.get_path_at_pos (tx, ty, out path, null, null, null);
275+ return path;
276+ }
277+
278+ protected void block_drag_and_drop () {
279+ tree_view.unset_rows_drag_source ();
280+ dnd_disabled = true;
281+ }
282+
283+ protected void unblock_drag_and_drop () {
284+ tree_view.enable_model_drag_source (Gdk.ModifierType.BUTTON1_MASK,
285+ source_targets,
286+ Gdk.DragAction.MOVE);
287+ dnd_disabled = false;
288+ }
289+
290 }
291 }

Subscribers

People subscribed via source and target branches

to all changes: