Merge lp:~elementary-apps/granite/dynamicnotebook into lp:~elementary-pantheon/granite/granite

Proposed by Rico Tzschichholz on 2013-04-09
Status: Work in progress
Proposed branch: lp:~elementary-apps/granite/dynamicnotebook
Merge into: lp:~elementary-pantheon/granite/granite
Diff against target: 323 lines (+147/-73)
1 file modified
lib/Widgets/DynamicNotebook.vala (+147/-73)
To merge this branch: bzr merge lp:~elementary-apps/granite/dynamicnotebook
Reviewer Review Type Date Requested Status
Cody Garver 2013-04-09 Needs Information on 2013-06-16
Tom Beckmann 2013-04-09 Pending
Review via email: mp+157874@code.launchpad.net

Description of the change

Just here to see a nice diff against trunk.

To post a comment you must log in.
559. By Rico Tzschichholz on 2013-04-10

Move dummy new_window callback for tab

Cody Garver (codygarver) wrote :

What's the hold up here?

David Gomes (davidgomes) wrote :

It's not perfect yet. I've been wanting to work on time but I still didn't have enough free time.

Cody Garver (codygarver) wrote :

Should the status of this branch be Rejected now that xapantu has got a DN fix in trunk?

review: Needs Information
xapantu (xapantu) wrote :

This branch does not fix the same bug, while my code just fixed the current bug, this is a larger refactoring of the way the DynamicNotebook works, which is interesting since this way is much more logical. The tab_moved signal is really not practical (the only good point in it is that everything can be handled with one signal, but I am not sure it is a very good idea).

Unmerged revisions

559. By Rico Tzschichholz on 2013-04-10

Move dummy new_window callback for tab

558. By Rico Tzschichholz on 2013-04-09

Avoid breaking ABI/API

557. By Tom Beckmann on 2013-03-29

Do proper signal handling fixing detaching tabs

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/Widgets/DynamicNotebook.vala'
2--- lib/Widgets/DynamicNotebook.vala 2013-04-05 07:11:07 +0000
3+++ lib/Widgets/DynamicNotebook.vala 2013-04-10 09:31:27 +0000
4@@ -314,13 +314,40 @@
5 private int tab_width = 150;
6 private int max_tab_width = 150;
7
8+ /**
9+ * A new tab was added
10+ * @param tab the new Tab
11+ */
12 public signal void tab_added (Tab tab);
13+
14+ /**
15+ * A new tab was added
16+ * Note: The return value has no meaning!
17+ * @param tab the removed Tab
18+ */
19 public signal bool tab_removed (Tab tab);
20- Tab? old_tab; //stores a reference for tab_switched
21+
22+ unowned Tab? old_tab; //stores a reference for tab_switched
23 public signal void tab_switched (Tab? old_tab, Tab new_tab);
24+
25+ //FIXME can be removed on API cleaning
26+ /**
27+ * Connect to tab_create_window and tab_reordered instead
28+ */
29+ [Deprecated (since=0.2)]
30 public signal void tab_moved (Tab tab, int new_pos, bool new_window, int x, int y);
31+
32 public signal void tab_duplicated (Tab duplicated_tab);
33
34+ public signal void tab_reordered (Tab tab, int new_pos);
35+
36+ /**
37+ * A tab was detached. You're supposed to connect to this signal
38+ * and return the notebook this tab should be added or null, if
39+ * you want to handle adding it manually
40+ */
41+ public signal DynamicNotebook? tab_detached (Tab tab, int x, int y);
42+
43 private static const string CLOSE_BUTTON_STYLE = """
44 * {
45 -GtkButton-default-border : 0;
46@@ -366,14 +393,12 @@
47 new_tab_m.activate.connect (() => {
48 var t = new Tab ();
49 notebook.page = (int) this.insert_tab (t, -1);
50- this.tab_added (t);
51 });
52
53 this.button_press_event.connect ((e) => {
54 if (e.type == Gdk.EventType.2BUTTON_PRESS && e.button == 1) {
55 var t = new Tab ();
56 notebook.page = (int) this.insert_tab (t, -1);
57- this.tab_added (t);
58 } else if (e.button == 3) {
59 menu.popup (null, null, null, 3, e.time);
60 }
61@@ -393,7 +418,6 @@
62 add.clicked.connect ( () => {
63 var t = new Tab ();
64 notebook.page = (int) this.insert_tab (t, -1);
65- this.tab_added (t);
66 });
67
68 this.size_allocate.connect (() => {
69@@ -416,7 +440,6 @@
70 case Gdk.Key.@T:
71 if ((e.state & Gdk.ModifierType.CONTROL_MASK) == Gdk.ModifierType.CONTROL_MASK) {
72 var t = new Tab ();
73- this.tab_added (t);
74 notebook.page = (int) this.insert_tab (t, -1);
75 return true;
76 }
77@@ -467,35 +490,80 @@
78 return false;
79 });
80
81+ notebook.page_added.connect (on_page_added);
82+ notebook.page_removed.connect (on_page_removed);
83 notebook.switch_page.connect (on_switch_page);
84 notebook.page_reordered.connect (on_page_reordered);
85 notebook.create_window.connect (on_create_window);
86 }
87
88 ~Notebook () {
89+ notebook.page_added.disconnect (on_page_added);
90+ notebook.page_removed.disconnect (on_page_removed);
91 notebook.switch_page.disconnect (on_switch_page);
92 notebook.page_reordered.disconnect (on_page_reordered);
93 notebook.create_window.disconnect (on_create_window);
94 }
95
96- void on_switch_page (Gtk.Widget page, uint pagenum) {
97- var new_tab = notebook.get_tab_label (page) as Tab;
98+ void on_switch_page (Gtk.Notebook notebook, Gtk.Widget page, uint pagenum) {
99+ unowned Tab? new_tab = notebook.get_tab_label (page) as Tab;
100+ if (new_tab == null)
101+ return;
102
103- tab_switched (old_tab, new_tab);
104+ unowned Tab? current_tab = old_tab as Tab;
105 old_tab = new_tab;
106- }
107-
108- void on_page_reordered (Gtk.Widget page, uint pagenum) {
109- tab_moved (notebook.get_tab_label (page) as Tab, (int) pagenum, false, -1, -1);
110+
111+ tab_switched (current_tab, new_tab);
112+ }
113+
114+ void on_page_reordered (Gtk.Notebook notebook, Gtk.Widget page, uint pagenum) {
115+ unowned Tab? tab = notebook.get_tab_label (page) as Tab;
116+ if (tab == null)
117+ return;
118+
119+ //FIXME must be removed on API cleaning
120+ if (Signal.has_handler_pending (this, Signal.lookup ("tab-moved", typeof (DynamicNotebook)), 0, true)) {
121+ // Filled up with dummy values to preserve API
122+ tab_moved (tab, (int)pagenum, false, 0, 0);
123+ return;
124+ }
125+
126+ tab_reordered (tab, (int)pagenum);
127+ }
128+
129+ void on_page_removed (Gtk.Notebook notebook, Gtk.Widget page, uint pagenum) {
130+ unowned Tab? tab = page as Tab;
131+ if (tab == null)
132+ return;
133+
134+ disconnect_tab_signals (tab);
135+ tab_removed (tab);
136+ }
137+
138+ void on_page_added (Gtk.Notebook notebook, Gtk.Widget page, uint pagenum) {
139+ unowned Tab? tab = notebook.get_tab_label (page) as Tab;
140+ if (tab == null)
141+ return;
142+
143+ connect_tab_signals (tab);
144+ tab_added (tab);
145 }
146
147 unowned Gtk.Notebook on_create_window (Gtk.Widget page, int x, int y) {
148- var tab = notebook.get_tab_label (page) as Tab;
149- notebook.remove_page (notebook.page_num (tab.page_container));
150- tab.page_container.destroy ();
151-
152- tab_moved (tab, 0, true, x, y);
153- return null;
154+ Tab? tab = notebook.get_tab_label (page) as Tab;
155+ if (tab == null)
156+ return null;
157+
158+ remove_tab (tab);
159+
160+ //FIXME must be removed on API cleaning
161+ if (Signal.has_handler_pending (this, Signal.lookup ("tab-moved", typeof (DynamicNotebook)), 0, true)) {
162+ tab_moved (tab, 0, true, x, y);
163+ return null;
164+ }
165+
166+ var new_notebook = tab_detached (notebook.get_tab_label (page) as Tab, x, y);
167+ return new_notebook != null ? new_notebook.notebook : null;
168 }
169
170 private void recalc_size () {
171@@ -516,25 +584,19 @@
172 }
173
174 public void remove_tab (Tab tab) {
175- if (Signal.has_handler_pending (this, Signal.lookup ("tab-removed", typeof (DynamicNotebook)), 0, true)) {
176- var sure = tab_removed (tab);
177- if (!sure)
178- return;
179- }
180-
181 var pos = get_tab_position (tab);
182- if (pos != -1)
183- notebook.remove_page (pos);
184- tab.page_container.destroy ();
185+ if (pos < 0)
186+ return;
187+
188+ notebook.remove_page (pos);
189 }
190
191 public void next_page () {
192- this.notebook.page = this.notebook.page + 1 >= this.notebook.get_n_pages () ? this.notebook.page = 0 : this.notebook.page + 1;
193+ notebook.next_page ();
194 }
195
196 public void previous_page () {
197- this.notebook.page = this.notebook.page - 1 < 0 ?
198- this.notebook.page = this.notebook.get_n_pages () - 1 : this.notebook.page - 1;
199+ notebook.prev_page ();
200 }
201
202 public override void show () {
203@@ -558,7 +620,6 @@
204
205 public void set_tab_position (Tab tab, int position) {
206 notebook.reorder_child (tab.page_container, position);
207- tab_moved (tab, position, false, -1, -1);
208 }
209
210 public Tab? get_tab_by_index (int index) {
211@@ -576,15 +637,6 @@
212 public uint insert_tab (Tab tab, int index) {
213 return_if_fail (tabs.index (tab) < 0);
214
215- var i = 0;
216- if (index == -1)
217- i = this.notebook.insert_page (tab.page_container, tab, this.notebook.get_n_pages ());
218- else
219- i = this.notebook.insert_page (tab.page_container, tab, index);
220-
221- this.notebook.set_tab_reorderable (tab.page_container, this.allow_drag);
222- this.notebook.set_tab_detachable (tab.page_container, this.allow_new_window);
223-
224 tab._icon.visible = show_icons;
225 tab.duplicate_m.visible = allow_duplication;
226 tab.new_window_m.visible = allow_new_window;
227@@ -592,41 +644,63 @@
228 tab.width_request = tab_width;
229 tab.close.get_style_context ().add_provider (button_fix,
230 Gtk.STYLE_PROVIDER_PRIORITY_APPLICATION);
231-
232- tab.closed.connect ( () => {
233- remove_tab (tab);
234- });
235-
236- tab.close_others.connect ( () => {
237- var num = 0; //save num, in case a tab refused to close so we don't end up in an infinite loop
238-
239- for (var j = 0; j < tabs.length (); j++) {
240- if (tab != tabs.nth_data (j)) {
241- tabs.nth_data (j).closed ();
242- if (num == n_tabs) break;
243- j--;
244- }
245-
246- num = n_tabs;
247- }
248- });
249-
250- tab.new_window.connect (() => {
251- notebook.remove_page (notebook.page_num (tab.page_container));
252- tab.page_container.destroy ();
253- tab_moved (tab, 0, true, 0, 0);
254- });
255-
256- tab.duplicate.connect (() => {
257- tab_duplicated (tab);
258- });
259-
260- this.recalc_size ();
261-
262- if (!tabs_closable)
263- tab.close.visible = false;
264+ tab.close.visible = tabs_closable;
265+
266+ uint i = 0;
267+ if (index == -1)
268+ i = notebook.insert_page (tab.page_container, tab, notebook.get_n_pages ());
269+ else
270+ i = notebook.insert_page (tab.page_container, tab, index);
271+
272+ notebook.set_tab_reorderable (tab.page_container, allow_drag);
273+ notebook.set_tab_detachable (tab.page_container, allow_new_window);
274+
275+ recalc_size ();
276
277 return i;
278 }
279+
280+ void connect_tab_signals (Tab tab) {
281+ tab.closed.connect (on_tab_closed);
282+ tab.close_others.connect (on_close_others);
283+ tab.new_window.connect (on_tab_new_window);
284+ tab.duplicate.connect (on_tab_duplicated);
285+ }
286+
287+ void disconnect_tab_signals (Tab tab) {
288+ tab.closed.disconnect (on_tab_closed);
289+ tab.close_others.disconnect (on_close_others);
290+ tab.new_window.disconnect (on_tab_new_window);
291+ tab.duplicate.disconnect (on_tab_duplicated);
292+ }
293+
294+ void on_tab_duplicated (Tab tab) {
295+ tab_duplicated (tab);
296+ }
297+
298+ void on_tab_new_window (Tab tab) {
299+ //notebook.remove_page (notebook.page_num (tab.page_container));
300+ //tab.page_container.destroy ();
301+ //notebook.create_window (tab.page_container, -1, -1);
302+ }
303+
304+ void on_tab_closed (Tab tab) {
305+ remove_tab (tab);
306+ }
307+
308+ void on_close_others (Tab tab) {
309+ var num = 0; //save num, in case a tab refused to close so we don't end up in an infinite loop
310+
311+ for (var j = 0; j < tabs.length (); j++) {
312+ unowned Tab t = tabs.nth_data (j);
313+ if (tab != t) {
314+ t.closed ();
315+ if (num == n_tabs) break;
316+ j--;
317+ }
318+
319+ num = n_tabs;
320+ }
321+ }
322 }
323 }

Subscribers

People subscribed via source and target branches