Merge lp:~vikoadi/scratch/fullscreen-toolbar into lp:~elementary-apps/scratch/scratch

Proposed by Viko Adi Rahmawan
Status: Merged
Approved by: Cody Garver
Approved revision: 1321
Merged at revision: 1320
Proposed branch: lp:~vikoadi/scratch/fullscreen-toolbar
Merge into: lp:~elementary-apps/scratch/scratch
Diff against target: 153 lines (+20/-14)
8 files modified
plugins/folder-manager/FolderManagerPlugin.vala (+2/-1)
plugins/pastebin/pastebin.vala (+2/-1)
plugins/pastebin/pastebin_dialog.vala (+3/-1)
src/Dialogs/PreferencesDialog.vala (+4/-2)
src/MainWindow.vala (+5/-4)
src/Services/Document.vala (+1/-1)
src/Utils.vala (+2/-2)
src/scratch-ui.xml (+1/-2)
To merge this branch: bzr merge lp:~vikoadi/scratch/fullscreen-toolbar
Reviewer Review Type Date Requested Status
Danielle Foré Needs Fixing
Robert Roth (community) code review Approve
Review via email: mp+222239@code.launchpad.net

Commit message

Remove fullscreen from appmenu but still leave F11 Fullscreen key
Window state saving fix

Description of the change

remove fullscreen from appmenu
also fix scratch to save Gdk.WindowState properly

To post a comment you must log in.
1312. By Viko Adi Rahmawan

fix codestyle

Revision history for this message
Robert Roth (evfool) wrote :

Thanks for the proposal, the fullscreen header is mostly working.
However, when opening the preferences while in fullscreen, the preferences dialog is shown, and the main window is hidden, and it does not reappear when the preferences dialog is closed, have to switch back to the application with Alt+Tab.
Could you please fix this?

review: Needs Fixing
1313. By Viko Adi Rahmawan

set all dialog parent or transient it

Revision history for this message
Viko Adi Rahmawan (vikoadi) wrote :

To fix those behaviour, i have to set all dialog's parent, dont know why it doesnt in the first place.
I also need to set all Plugins Dialog to transient (Pastebin, FolderManager)
Please tell me if i miss something

1314. By Viko Adi Rahmawan

fix codestyle

Revision history for this message
Robert Roth (evfool) wrote :

Setting transient for doesn't seem to be enough for preferences and pastebin dialog, they should have a parent set too, I guess.

Revision history for this message
Viko Adi Rahmawan (vikoadi) wrote :

what behaviour that still lacking? are you refering to modal behaviour?

Revision history for this message
Robert Roth (evfool) wrote :

My testcase is as follows: open scratch, fullscreen it, press the Super key for overview. (on gnome shell, I'm not on gala atm)
E.g. in gnome-shell overview before marking as transient the dialogs have appeared as separate windows, now they don't appear as transient, but in the overview the dialogs are not shown over the scratch window, yet, when you select scratch, you get the dialog as active.

1315. By Viko Adi Rahmawan

set dialog to modal

Revision history for this message
Viko Adi Rahmawan (vikoadi) wrote :

i think that was the problem with wm, i dont think, mutter will overlay dialog on top of window in overview. can you try the open file dialog?
does modal dialog working?

Revision history for this message
Robert Roth (evfool) wrote :

@vikoadi: It might indeed be a problem with my WM, better leave the dialogs not modal (based on elementary HIG), only transient, so the last change should be reverted.

1316. By Viko Adi Rahmawan

revert modal

Revision history for this message
Viko Adi Rahmawan (vikoadi) wrote :

i revert the changes

Revision history for this message
Robert Roth (evfool) wrote :

One last comment inline before approving.

1317. By Viko Adi Rahmawan

codestyle fix

1318. By Viko Adi Rahmawan

use Gtk.Window instead

Revision history for this message
Viko Adi Rahmawan (vikoadi) wrote :

use Gtk.Window

Revision history for this message
Robert Roth (evfool) :
review: Approve (code review)
Revision history for this message
Danielle Foré (danrabbit) wrote :

Alright, so I've got a screencast showing some really odd behavior for the revealer: https://ele.slack.com/files/danrabbit/F02CRVCA5/screencast1404370511.webm

I'm not sure I like that the window controls are no longer present in the header. In Audience you still have an unmaximize on the right side, I would expect to see it here too.

If you click the gear menu, the menu still shows but the header disappears.

In my personal opinion, the fullscreen feature isn't worth the added complexity. We're talking about literally 4 more lines of code. We should just remove the menu item and say "hey if you use the shortcut, more power to you". Would be the simpler fix.

review: Needs Fixing
Revision history for this message
Viko Adi Rahmawan (vikoadi) wrote :

I only tried to replicate how gedit do their full screen, I'll make it whatever design decision anyway.

1319. By Viko Adi Rahmawan

remove fullscreen from appmenu, remove all sliding overlay headerbar

Revision history for this message
Viko Adi Rahmawan (vikoadi) wrote :

If you want to put headerbar in fullscreen mode, we cant do that without much hack. its wm which decide to hide the headerbar.

1320. By Viko Adi Rahmawan

little fix

Revision history for this message
Cody Garver (codygarver) wrote :

Fullscreen got dropped entirely by another merge

Revision history for this message
Cody Garver (codygarver) wrote :

Ignore what I said and did earlier, Fullscreen still exists. But Dan thinks it would be best to just drop it entirely.

Feel free to overwrite this branch with such a solution.

1321. By Viko Adi Rahmawan

fix codestyle

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/folder-manager/FolderManagerPlugin.vala'
2--- plugins/folder-manager/FolderManagerPlugin.vala 2013-11-30 22:43:00 +0000
3+++ plugins/folder-manager/FolderManagerPlugin.vala 2014-07-12 05:58:52 +0000
4@@ -89,8 +89,9 @@
5 tool_button = new Gtk.ToolButton (icon, _("Open a folder"));
6 tool_button.tooltip_text = _("Open a folder");
7 tool_button.clicked.connect (() => {
8+ Gtk.Window window = plugins.manager.window;
9 Gtk.FileChooserDialog chooser = new Gtk.FileChooserDialog (
10- "Select a folder.", null, Gtk.FileChooserAction.SELECT_FOLDER,
11+ "Select a folder.", window, Gtk.FileChooserAction.SELECT_FOLDER,
12 Gtk.Stock.CANCEL, Gtk.ResponseType.CANCEL,
13 Gtk.Stock.OPEN, Gtk.ResponseType.ACCEPT);
14 chooser.select_multiple = true;
15
16=== modified file 'plugins/pastebin/pastebin.vala'
17--- plugins/pastebin/pastebin.vala 2013-06-07 21:00:59 +0000
18+++ plugins/pastebin/pastebin.vala 2014-07-12 05:58:52 +0000
19@@ -133,7 +133,8 @@
20 menuitem.destroy ();
21 menuitem = new Gtk.MenuItem.with_label (_("Upload to Pastebin"));
22 menuitem.activate.connect (() => {
23- new Dialogs.PasteBinDialog (doc);
24+ MainWindow window = plugins.manager.window;
25+ new Dialogs.PasteBinDialog (window, doc);
26 });
27 menu.append (menuitem);
28 menuitem.show_all ();
29
30=== modified file 'plugins/pastebin/pastebin_dialog.vala'
31--- plugins/pastebin/pastebin_dialog.vala 2013-11-29 20:39:44 +0000
32+++ plugins/pastebin/pastebin_dialog.vala 2014-07-12 05:58:52 +0000
33@@ -246,9 +246,11 @@
34 private Button send_button;
35
36
37- public PasteBinDialog (Scratch.Services.Document doc) {
38+ public PasteBinDialog (Gtk.Window? parent, Scratch.Services.Document doc) {
39 this.doc = doc;
40
41+ if (parent != null)
42+ this.set_transient_for (parent);
43 this.title = _("Share via PasteBin");
44 this.type_hint = Gdk.WindowTypeHint.DIALOG;
45
46
47=== modified file 'src/Dialogs/PreferencesDialog.vala'
48--- src/Dialogs/PreferencesDialog.vala 2014-04-13 16:12:53 +0000
49+++ src/Dialogs/PreferencesDialog.vala 2014-07-12 05:58:52 +0000
50@@ -41,8 +41,10 @@
51 Switch use_custom_font;
52 FontButton select_font;
53
54- public Preferences (PluginsManager plugins) {
55+ public Preferences (Gtk.Window? parent, PluginsManager plugins) {
56
57+ if (parent != null)
58+ this.set_transient_for (parent);
59 this.title = _("Preferences");
60 this.border_width = 5;
61 set_default_size (630, 330);
62@@ -278,4 +280,4 @@
63
64 }
65
66-} // Namespace
67\ No newline at end of file
68+} // Namespace
69
70=== modified file 'src/MainWindow.vala'
71--- src/MainWindow.vala 2014-06-04 20:11:34 +0000
72+++ src/MainWindow.vala 2014-07-12 05:58:52 +0000
73@@ -71,6 +71,7 @@
74 this.title = this.app.app_cmd_name;
75 this.window_position = Gtk.WindowPosition.CENTER;
76 this.set_size_request (450, 400);
77+ this.set_hide_titlebar_when_maximized (false);
78 restore_saved_state ();
79 this.icon_name = "accessories-text-editor";
80
81@@ -419,9 +420,9 @@
82 private void update_saved_state () {
83
84 // Save window state
85- if (get_window ().get_state () == WindowState.MAXIMIZED)
86+ if ((get_window ().get_state () & WindowState.MAXIMIZED) != 0)
87 Scratch.saved_state.window_state = ScratchWindowState.MAXIMIZED;
88- else if (get_window ().get_state () == WindowState.FULLSCREEN)
89+ else if ((get_window ().get_state () & WindowState.FULLSCREEN) != 0)
90 Scratch.saved_state.window_state = ScratchWindowState.FULLSCREEN;
91 else
92 Scratch.saved_state.window_state = ScratchWindowState.NORMAL;
93@@ -479,7 +480,7 @@
94
95 // Actions functions
96 void action_preferences () {
97- var dialog = new Scratch.Dialogs.Preferences (plugins);
98+ var dialog = new Scratch.Dialogs.Preferences (this, plugins);
99 dialog.show_all ();
100 }
101
102@@ -499,7 +500,7 @@
103
104 void action_open () {
105 // Show a GtkFileChooserDialog
106- var filech = Utils.new_file_chooser_dialog (FileChooserAction.OPEN, _("Open some files"), true);
107+ var filech = Utils.new_file_chooser_dialog (FileChooserAction.OPEN, _("Open some files"), this, true);
108
109 if (filech.run () == ResponseType.ACCEPT) {
110 foreach (string uri in filech.get_uris ()) {
111
112=== modified file 'src/Services/Document.vala'
113--- src/Services/Document.vala 2014-07-11 19:55:41 +0000
114+++ src/Services/Document.vala 2014-07-12 05:58:52 +0000
115@@ -327,7 +327,7 @@
116
117 public bool save_as () {
118 // New file
119- var filech = Utils.new_file_chooser_dialog (Gtk.FileChooserAction.SAVE, _("Save File"));
120+ var filech = Utils.new_file_chooser_dialog (Gtk.FileChooserAction.SAVE, _("Save File"), null);
121
122 if (filech.run () == Gtk.ResponseType.ACCEPT) {
123 this.file = File.new_for_uri (filech.get_file ().get_uri ());
124
125=== modified file 'src/Utils.vala'
126--- src/Utils.vala 2013-05-10 14:10:59 +0000
127+++ src/Utils.vala 2014-07-12 05:58:52 +0000
128@@ -23,8 +23,8 @@
129 public string? last_path = null;
130
131 // Create a GtkFileChooserDialog to perform the action desired
132- public Gtk.FileChooserDialog new_file_chooser_dialog (Gtk.FileChooserAction action, string title, bool select_multiple = false) {
133- var filech = new Gtk.FileChooserDialog (title, null, action);
134+ public Gtk.FileChooserDialog new_file_chooser_dialog (Gtk.FileChooserAction action, string title, Gtk.Window? parent, bool select_multiple = false) {
135+ var filech = new Gtk.FileChooserDialog (title, parent, action);
136 filech.set_select_multiple (select_multiple);
137 filech.add_button (Gtk.Stock.CANCEL, Gtk.ResponseType.CANCEL);
138 if (action == Gtk.FileChooserAction.OPEN)
139
140=== modified file 'src/scratch-ui.xml'
141--- src/scratch-ui.xml 2013-12-15 16:22:24 +0000
142+++ src/scratch-ui.xml 2014-07-12 05:58:52 +0000
143@@ -24,8 +24,7 @@
144 <popup name="AppMenu">
145 <menuitem action="NewView" />
146 <menuitem action="RemoveView" />
147- <menuitem action="Fullscreen" />
148 <separator />
149 <menuitem action="Preferences" />
150 </popup>
151-</ui>
152\ No newline at end of file
153+</ui>

Subscribers

People subscribed via source and target branches