Merge lp:~superscript18/scratch/fix-1013421 into lp:~elementary-apps/scratch/scratch

Proposed by SuperScript
Status: Merged
Approved by: Artem Anufrij
Approved revision: 1425
Merged at revision: 1423
Proposed branch: lp:~superscript18/scratch/fix-1013421
Merge into: lp:~elementary-apps/scratch/scratch
Diff against target: 106 lines (+51/-8)
1 file modified
plugins/terminal/terminal.vala (+51/-8)
To merge this branch: bzr merge lp:~superscript18/scratch/fix-1013421
Reviewer Review Type Date Requested Status
Artem Anufrij (community) code & ux Approve
Review via email: mp+243059@code.launchpad.net

Commit message

Added Terminal plugin toggle button. Fixes bug 1013421

Description of the change

Added Terminal plugin toggle button. Fixes bug 1013421

To post a comment you must log in.
1419. By SuperScript

Fixed not removing icon with plugin. Also not showing is default now.

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

Please resolve the code conflicts.
(brz merge lp:scratch)

Caution: Terminal can be switched between the bottombar or contextbar (right).

https://bugs.launchpad.net/scratch/+bug/1085718

review: Needs Fixing (code)
1420. By SuperScript

Trying to fix with new sidebar stuff.

1421. By SuperScript

Merged in code for right-side switch. Now working again.

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

Tooltip should switch between "Show terminal" and "hide Terminal" like browser toggle button.

See line comments

review: Needs Fixing (code)
Revision history for this message
Artem Anufrij (artem-anufrij) wrote :
1422. By SuperScript

Made the tooltip toggle between show and hide.

1423. By SuperScript

Made the tooltip toggle between show and hide.

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

After "Show Terminal" Terminal should be showed in the foreground.

See my screencast:
https://bugs.launchpad.net/scratch/+bug/1013421/+attachment/4273655/+files/Screencast%202014-12-03%2020%3A41%3A01.mp4

review: Needs Fixing (ux)
1424. By SuperScript

Terminal goes to foreground now.

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

line comments

review: Needs Fixing (code style)
1425. By SuperScript

Fixed coding style, removed junk.

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

Works as expected. Code style was reviewed!

Great work! Thank you!

review: Approve (code & ux)

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-30 23:20:55 +0000
3+++ plugins/terminal/terminal.vala 2014-12-03 20:42:43 +0000
4@@ -29,7 +29,11 @@
5
6 Gtk.Notebook? bottombar = null;
7 Gtk.Notebook? contextbar = null;
8-
9+ Scratch.Widgets.Toolbar? toolbar = null;
10+ Gtk.ToggleToolButton? toolbutton = null;
11+
12+ bool on_bottom = true;
13+
14 Gtk.RadioMenuItem location_bottom = null;
15 Gtk.RadioMenuItem location_right = null;
16
17@@ -58,7 +62,6 @@
18 if (bottombar == null) {
19 this.bottombar = n;
20 }
21- switch_terminal_location ();
22 });
23
24 plugins.hook_notebook_context.connect ((n) => {
25@@ -67,28 +70,39 @@
26 }
27 });
28
29- on_hook ();
30+ plugins.hook_toolbar.connect ((n) => {
31+ if (toolbar == null) {
32+ this.toolbar = n;
33+ on_hook_toolbar (this.toolbar);
34+ }
35+ });
36+
37+ on_hook_notebook ();
38 }
39
40 public void deactivate () {
41 if (terminal != null)
42 grid.destroy ();
43+ if (toolbutton != null)
44+ toolbutton.destroy ();
45
46 window.key_press_event.disconnect (switch_focus);
47 }
48
49 void switch_terminal_location () {
50-
51+
52 if (bottombar.page_num (grid) == -1 && this.location_bottom.active) {
53
54 contextbar.remove_page (contextbar.page_num (grid));
55- bottombar.append_page (grid, new Gtk.Label (_("Terminal")));
56+ bottombar.set_current_page (bottombar.append_page (grid, new Gtk.Label (_("Terminal"))));
57+ on_bottom = true;
58 debug ("Move Terminal: BOTTOMBAR.");
59-
60+
61 } else if (contextbar.page_num (grid) == -1 && this.location_right.active) {
62
63 bottombar.remove_page (bottombar.page_num (grid));
64- contextbar.append_page (grid, new Gtk.Label (_("Terminal")));
65+ contextbar.set_current_page (contextbar.append_page (grid, new Gtk.Label (_("Terminal"))));
66+ on_bottom = false;
67 debug ("Move Terminal: CONTEXTBAR.");
68
69 }
70@@ -116,7 +130,36 @@
71 return false;
72 }
73
74- void on_hook () {
75+ void on_hook_toolbar (Scratch.Widgets.Toolbar toolbar) {
76+ var icon = new Gtk.Image.from_icon_name ("utilities-terminal", Gtk.IconSize.LARGE_TOOLBAR);
77+ toolbutton = new Gtk.ToggleToolButton ();
78+ toolbutton.set_icon_widget (icon);
79+ toolbutton.set_active (false);
80+ toolbutton.tooltip_text = _("Show Terminal");
81+ toolbutton.toggled.connect (() => {
82+ if (this.toolbutton.active) {
83+ toolbutton.tooltip_text = _("Hide Terminal");
84+ if (on_bottom) {
85+ bottombar.set_current_page (bottombar.append_page (grid, new Gtk.Label (_("Terminal"))));
86+ } else {
87+ contextbar.set_current_page (contextbar.append_page (grid, new Gtk.Label (_("Terminal"))));
88+ }
89+ } else {
90+ toolbutton.tooltip_text = _("Show Terminal");
91+ if (on_bottom) {
92+ bottombar.remove_page (bottombar.page_num (grid));
93+ } else {
94+ contextbar.remove_page (contextbar.page_num (grid));
95+ }
96+ }
97+ });
98+
99+ toolbutton.show_all ();
100+
101+ toolbar.pack_start (toolbutton);
102+ }
103+
104+ void on_hook_notebook () {
105 this.terminal = new Vte.Terminal ();
106 this.terminal.scrollback_lines = -1;
107

Subscribers

People subscribed via source and target branches