Merge lp:~artem-anufrij/scratch/Bugfix-1085718 into lp:~elementary-apps/scratch/scratch

Proposed by Artem Anufrij
Status: Superseded
Proposed branch: lp:~artem-anufrij/scratch/Bugfix-1085718
Merge into: lp:~elementary-apps/scratch/scratch
Diff against target: 252 lines (+73/-21)
4 files modified
plugins/terminal/terminal.vala (+57/-16)
src/MainWindow.vala (+11/-3)
src/Widgets/SplitView.vala (+3/-1)
src/Widgets/ToolBar.vala (+2/-1)
To merge this branch: bzr merge lp:~artem-anufrij/scratch/Bugfix-1085718
Reviewer Review Type Date Requested Status
Danielle Foré ux Pending
Artem Anufrij (ux) Pending
Review via email: mp+241621@code.launchpad.net

This proposal supersedes a proposal from 2014-11-02.

This proposal has been superseded by a proposal from 2014-11-24.

Description of the change

Switching terminal location via context menu.

To post a comment you must log in.
Revision history for this message
Danielle Foré (danrabbit) wrote : Posted in a previous version of this proposal

I would prefer not to set a static px limit here. We should change based on the side of the Line Width Guide and also if there are other panes open like the Folder Browser.

review: Needs Fixing (ux)
Revision history for this message
Artem Anufrij (artem-anufrij) wrote : Posted in a previous version of this proposal

Hey Dan,

I didn't get any feedback on slack.com about this behavior. I use since a week this branch on my system and it's very helpful, although it will be checked the window width in pix, and not line width.

I think we can merge this branch.

What do you think about?

review: Needs Information ((ux))
Revision history for this message
Artem Anufrij (artem-anufrij) wrote :
Revision history for this message
Cody Garver (codygarver) wrote :

The terminal starts too slim, no? http://i.imgur.com/ODxhith.png

Revision history for this message
Artem Anufrij (artem-anufrij) wrote :

@Cody:
It's the currently width of the right bar.

Revision history for this message
Danielle Foré (danrabbit) wrote :

Yeah that's way too thin. And again, I don't think it's acceptable to hardcode a px size. We need to be smart about what a sane minimum size is for the terminal and also about what space we have available. hardcoding a px size doesn't take into account other plugins or when settings have been changed by the user.

Revision history for this message
Artem Anufrij (artem-anufrij) wrote :

Dan, what do you think about a button in the toolbar for manual switching the terminal location?

Revision history for this message
Danielle Foré (danrabbit) wrote :

If we can't do it automatically, perhaps a context menu on the Terminal would be a little more subtle.

1408. By artem-anufrij

Switching terminal location via context menu.

Revision history for this message
Artem Anufrij (artem-anufrij) wrote :

Could you please review the branch...

1409. By artem-anufrij

Change Menu items: Terminal on Right | Terminal on Bottom

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/terminal/terminal.vala'
2--- plugins/terminal/terminal.vala 2014-11-10 18:09:21 +0000
3+++ plugins/terminal/terminal.vala 2014-11-24 20:00:44 +0000
4@@ -26,7 +26,12 @@
5 public class Scratch.Plugins.Terminal : Peas.ExtensionBase, Peas.Activatable {
6
7 MainWindow window = null;
8+
9 Gtk.Notebook? bottombar = null;
10+ Gtk.Notebook? contextbar = null;
11+
12+ Gtk.MenuItem location = null;
13+
14 Vte.Terminal terminal;
15 Gtk.Grid grid;
16
17@@ -37,12 +42,13 @@
18 }
19
20 public void activate () {
21+
22 plugins = (Scratch.Services.Interface) object;
23
24 plugins.hook_window.connect ((w) => {
25 if (window != null)
26 return;
27-
28+
29 window = w;
30 window.key_press_event.connect (switch_focus);
31 });
32@@ -50,38 +56,68 @@
33 plugins.hook_notebook_bottom.connect ((n) => {
34 if (bottombar == null) {
35 this.bottombar = n;
36- on_hook (this.bottombar);
37- }
38- });
39- if (bottombar != null)
40- on_hook (this.bottombar);
41+ }
42+ switch_terminal_location ();
43+ });
44+
45+ plugins.hook_notebook_context.connect ((n) => {
46+ if (contextbar == null) {
47+ this.contextbar = n;
48+ }
49+ });
50+
51+ on_hook ();
52 }
53
54 public void deactivate () {
55 if (terminal != null)
56 grid.destroy ();
57- if (window != null)
58- window.key_press_event.disconnect (switch_focus);
59+
60+ window.key_press_event.disconnect (switch_focus);
61+ }
62+
63+ void switch_terminal_location () {
64+
65+ if (bottombar.page_num (grid) == -1) {
66+
67+ contextbar.remove_page (contextbar.page_num (grid));
68+ bottombar.append_page (grid, new Gtk.Label (_("Terminal")));
69+ location.set_label (_("Move Terminal into contextbar"));
70+ debug ("Move Terminal: BOTTOMBAR.");
71+
72+ } else if (contextbar.page_num (grid) == -1 ) {
73+
74+ bottombar.remove_page (bottombar.page_num (grid));
75+ contextbar.append_page (grid, new Gtk.Label (_("Terminal")));
76+ location.set_label (_("Move Terminal into bottombar"));
77+ debug ("Move Terminal: CONTEXTBAR.");
78+
79+ }
80 }
81
82 bool switch_focus (Gdk.EventKey event) {
83 if (event.keyval == Gdk.Key.t
84 && Gdk.ModifierType.MOD1_MASK in event.state
85 && Gdk.ModifierType.CONTROL_MASK in event.state) {
86+
87 if (terminal.has_focus && window.get_current_document () != null) {
88+
89 window.get_current_document ().focus ();
90 debug ("Move focus: EDITOR.");
91 return true;
92+
93 } else if (window.get_current_document () != null && window.get_current_document ().source_view.has_focus) {
94+
95 terminal.grab_focus ();
96 debug ("Move focus: TERMINAL.");
97 return true;
98+
99 }
100 }
101 return false;
102 }
103
104- void on_hook (Gtk.Notebook notebook) {
105+ void on_hook () {
106 this.terminal = new Vte.Terminal ();
107 this.terminal.scrollback_lines = -1;
108
109@@ -147,20 +183,28 @@
110 // Popup menu
111 var menu = new Gtk.Menu ();
112
113- var copy = new Gtk.MenuItem.with_label (_("Copy"));
114+ Gtk.MenuItem copy = new Gtk.MenuItem.with_label (_("Copy"));
115 copy.activate.connect (() => {
116 terminal.copy_clipboard ();
117 });
118 menu.append (copy);
119 copy.show ();
120
121- var paste = new Gtk.MenuItem.with_label (_("Paste"));
122+ Gtk.MenuItem paste = new Gtk.MenuItem.with_label (_("Paste"));
123 paste.activate.connect (() => {
124 terminal.paste_clipboard ();
125 });
126 menu.append (paste);
127 paste.show ();
128
129+ location = new Gtk.MenuItem.with_label ("");
130+ location.activate.connect (() => {
131+ switch_terminal_location ();
132+ });
133+
134+ menu.append (location);
135+ location.show ();
136+
137 this.terminal.button_press_event.connect ((event) => {
138 if (event.button == 3) {
139 menu.select_first (false);
140@@ -168,7 +212,7 @@
141 }
142 return false;
143 });
144-
145+
146 try {
147 this.terminal.fork_command_full (Vte.PtyFlags.DEFAULT, "~/", { Vte.get_user_shell () }, null, GLib.SpawnFlags.SEARCH_PATH, null, null);
148 } catch (GLib.Error e) {
149@@ -184,10 +228,7 @@
150 terminal.vexpand = true;
151 terminal.hexpand = true;
152
153- notebook.append_page (grid, new Gtk.Label (_("Terminal")));
154-
155 grid.show_all ();
156-
157 }
158 }
159
160@@ -196,4 +237,4 @@
161 var objmodule = module as Peas.ObjectModule;
162 objmodule.register_extension_type (typeof (Peas.Activatable),
163 typeof (Scratch.Plugins.Terminal));
164-}
165\ No newline at end of file
166+}
167
168=== modified file 'src/MainWindow.vala'
169--- src/MainWindow.vala 2014-11-23 10:44:26 +0000
170+++ src/MainWindow.vala 2014-11-24 20:00:44 +0000
171@@ -214,13 +214,21 @@
172
173 this.contextbar = new Gtk.Notebook ();
174 this.contextbar.no_show_all = true;
175- this.contextbar.page_added.connect (() => { on_plugin_toggled (contextbar); });
176 this.contextbar.page_removed.connect (() => { on_plugin_toggled (contextbar); });
177+ this.contextbar.page_added.connect (() => {
178+ if (!this.split_view.is_empty ())
179+ on_plugin_toggled (contextbar);
180+ });
181+
182+
183
184 this.bottombar = new Gtk.Notebook ();
185 this.bottombar.no_show_all = true;
186- this.bottombar.page_added.connect (() => { on_plugin_toggled (bottombar); });
187 this.bottombar.page_removed.connect (() => { on_plugin_toggled (bottombar); });
188+ this.bottombar.page_added.connect (() => {
189+ if (!this.split_view.is_empty ())
190+ on_plugin_toggled (bottombar);
191+ });
192
193 hp1 = new Granite.Widgets.ThinPaned ();
194 hp2 = new Granite.Widgets.ThinPaned ();
195@@ -287,7 +295,7 @@
196 set_widgets_sensitive (!split_view.is_empty ());
197 }
198
199- private void on_plugin_toggled (Gtk.Notebook notebook) {
200+ private void on_plugin_toggled (Gtk.Notebook notebook) {
201 var pages = notebook.get_n_pages ();
202 notebook.set_show_tabs (pages > 1);
203 notebook.no_show_all = (pages == 0);
204
205=== modified file 'src/Widgets/SplitView.vala'
206--- src/Widgets/SplitView.vala 2014-10-26 08:26:36 +0000
207+++ src/Widgets/SplitView.vala 2014-11-24 20:00:44 +0000
208@@ -35,6 +35,7 @@
209 public signal void welcome_shown ();
210 public signal void welcome_hidden ();
211 public signal void document_change (Scratch.Services.Document document);
212+ public signal void views_changed (uint count);
213
214 private weak MainWindow window;
215
216@@ -126,7 +127,6 @@
217
218 // Enbale/Disable useless GtkActions about views
219 check_actions ();
220-
221 return view;
222 }
223
224@@ -199,6 +199,8 @@
225 private void check_actions () {
226 window.main_actions.get_action ("NewView").sensitive = (views.length () < 2);
227 window.main_actions.get_action ("RemoveView").sensitive = (views.length () > 1);
228+
229+ views_changed (views.length ());
230 }
231 }
232 }
233\ No newline at end of file
234
235=== modified file 'src/Widgets/ToolBar.vala'
236--- src/Widgets/ToolBar.vala 2014-10-28 18:20:18 +0000
237+++ src/Widgets/ToolBar.vala 2014-11-24 20:00:44 +0000
238@@ -91,6 +91,7 @@
239 pack_start (revert_button);
240 pack_start (new SeparatorToolItem ());
241 pack_start (find_button);
242+ pack_start (find_button);
243
244 pack_end (share_app_menu);
245 pack_end (zoom_default);
246@@ -106,4 +107,4 @@
247
248 }
249 }
250-}
251\ No newline at end of file
252+}

Subscribers

People subscribed via source and target branches