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