Merge lp:~teemperor/simple-scan/elementary-ui into lp:~simple-scan-team/simple-scan/trunk

Proposed by Raphael Isemann
Status: Rejected
Rejected by: Robert Ancell
Proposed branch: lp:~teemperor/simple-scan/elementary-ui
Merge into: lp:~simple-scan-team/simple-scan/trunk
Diff against target: 148 lines (+74/-5)
3 files modified
configure.ac (+1/-0)
src/Makefile.am (+2/-1)
src/ui.vala (+71/-4)
To merge this branch: bzr merge lp:~teemperor/simple-scan/elementary-ui
Reviewer Review Type Date Requested Status
Robert Ancell Needs Fixing
Review via email: mp+147199@code.launchpad.net

Description of the change

I changed the UI as seen here ( https://lists.launchpad.net/elementary-dev-community/msg01568.html ) to meet the elementary-HIG.

My code shouldn't affect simple-scan on other desktops then pantheon in any way.

I get the DE-name from the DESKTOP_SESSION variable, so to test the changes from another distribution you need to run "export DESKTOP_SESSION=pantheon" before executing simple-scan (from the same shell as the export-command).

New dependencies are only the granite library.

For quick feedback i'm also available on #elementary-dev (freenode) as "teemperor".

Have a nice evening!

To post a comment you must log in.
Revision history for this message
Robert Ancell (robert-ancell) wrote :

So sorry! It appears I wasn't subscribed to merge requests so I didn't see this merge (I was expecting it though). Feel free to ping me directly if you think I've missed something like this.

Revision history for this message
Robert Ancell (robert-ancell) wrote :

What I like here is the removal of the menubar and the cog menu design - that is good to go upstream.

Issues with the branch as it stands:
- You have a hard dependency on granite - this needs to be optional.
- In saying that, I don't see the value in us taking a dependency on granite since it's Elementary specific. So I'd expect to get the majority of the functional changes into upstream simple-scan but still need a small patch for Elementary specific functionality.
- By removing the menu you have broken the right click functionality on a page.
- The cog menu is missing important functionality like email and print.
- You don't need the crop option in the cog menu (it is in right click on the page). Though it might be worth having the whole page menu as a submenu so it is discoverable.
- The about dialog renames and points at lp:elementary-scan - if this is upstreamed code it should point at lp:simple-scan
- Coding style doesn't match existing code (whitespace, comment style etc).
- Unnecessary changes in the branch - newline at the end of a function.

What I recommend you do is make this branch:
- Remove the current menubar (keeping the right click menu)
- Add the cog menu and populate with existing functionality.

We can then look in a second patch the value of adding an optional granite dependency.

review: Needs Fixing
Revision history for this message
Eduard Gotwig (gotwig) wrote :

this code is outdated, and the merge request should be removed.

Unmerged revisions

606. By Raphael Isemann

removed libgee from dependencies

605. By Raphael Isemann

removed the contractor-section that was already commented out

604. By Raphael Isemann

added an optional ui that meets the elementary-guidelines

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'configure.ac'
2--- configure.ac 2013-01-19 02:45:11 +0000
3+++ configure.ac 2013-02-07 18:13:26 +0000
4@@ -28,6 +28,7 @@
5 gdk-pixbuf-2.0
6 gudev-1.0
7 sqlite3
8+ granite
9 ])
10
11 PKG_CHECK_MODULES(COLORD, [
12
13=== modified file 'src/Makefile.am'
14--- src/Makefile.am 2012-02-03 20:49:21 +0000
15+++ src/Makefile.am 2013-02-07 18:13:26 +0000
16@@ -19,7 +19,8 @@
17 --pkg=gudev-1.0 \
18 --pkg=gio-2.0 \
19 --pkg=gtk+-3.0 \
20- --pkg=sqlite3
21+ --pkg=sqlite3 \
22+ --pkg=granite
23
24 if HAVE_COLORD
25 simple_scan_VALAFLAGS += -D HAVE_COLORD
26
27=== modified file 'src/ui.vala'
28--- src/ui.vala 2013-01-19 02:45:11 +0000
29+++ src/ui.vala 2013-02-07 18:13:26 +0000
30@@ -1,7 +1,7 @@
31 /*
32 * Copyright (C) 2009-2011 Canonical Ltd.
33- * Author: Robert Ancell <robert.ancell@canonical.com>
34- *
35+ * Authors: Robert Ancell <robert.ancell@canonical.com>
36+ * Raphael Isemann <teemperor@googlemail.com>
37 * This program is free software: you can redistribute it and/or modify it under
38 * the terms of the GNU General Public License as published by the Free Software
39 * Foundation, either version 3 of the License, or (at your option) any later
40@@ -92,6 +92,9 @@
41 private int window_height;
42 private bool window_is_maximized;
43
44+ //indicates if the current enviroment is the pantheon shell
45+ private bool is_pantheon;
46+
47 public signal void start_scan (string? device, ScanOptions options);
48 public signal void stop_scan ();
49 public signal void email (string profile);
50@@ -104,9 +107,13 @@
51
52 settings = new Settings ("org.gnome.SimpleScan");
53
54+ string? env = Environment.get_variable("DESKTOP_SESSION");
55+ is_pantheon = (env == "pantheon");
56+
57 load ();
58
59 autosave_manager = AutosaveManager.create (ref book);
60+
61 }
62
63 ~UserInterface ()
64@@ -1095,8 +1102,29 @@
65
66 /* Description of program */
67 string description = _("Simple document scanning tool");
68-
69- Gtk.show_about_dialog (window,
70+ if(is_pantheon)
71+ Granite.Widgets.show_about_dialog ((Gtk.Window) widget,
72+ "program_name", "Simple Scan with elementary UI",
73+ "version", VERSION,
74+ "logo_icon_name", "scanner",
75+
76+ "comments", description,
77+ "copyright", "2009-2011 Canonical Ltd.",
78+ "website", "https://launchpad.net/elementary-scan",
79+ "website_label", "Website",
80+
81+ "authors", authors,
82+ //"documenters", about_documenters,
83+ //"artists", about_artists,
84+ "translator_credits", _("translator-credits"),
85+ "license", license,
86+ "license_type", Gtk.License.GPL_3_0,
87+
88+ "help", "https://answers.launchpad.net/elementary-scan",
89+ "translate", "https://translations.launchpad.net/simple-scan",
90+ "bug", "https://bugs.launchpad.net/elementary-scan");
91+ else
92+ Gtk.show_about_dialog (window,
93 "title", title,
94 "program-name", "Simple Scan",
95 "version", VERSION,
96@@ -1109,6 +1137,7 @@
97 "license", license,
98 "wrap-license", true,
99 null);
100+
101 }
102
103 private bool on_quit ()
104@@ -1316,6 +1345,44 @@
105 photo_toolbar_menuitem = (Gtk.RadioMenuItem) builder.get_object ("photo_toolbutton_menuitem");
106 photo_menu_menuitem = (Gtk.RadioMenuItem) builder.get_object ("photo_menuitem");
107
108+ if(is_pantheon) {
109+ //hide the menubar to follow elementary-guidelines
110+ var menubar = (Gtk.MenuBar) builder.get_object ("menubar");
111+ menubar.hide();
112+
113+ //unparent menuitems that we want in the appmenu
114+ var crop_menuitem = (Gtk.MenuItem) builder.get_object ("crop_menuitem");
115+ crop_menuitem.unparent ();
116+ var content_menuitem = (Gtk.MenuItem) builder.get_object ("help_contents_menuitem");
117+ content_menuitem.unparent ();
118+ var about_menuitem = (Gtk.MenuItem) builder.get_object ("about_menuitem");
119+ about_menuitem.unparent ();
120+ var preferences_menuitem = (Gtk.MenuItem) builder.get_object ("preferences_menuitem");
121+ preferences_menuitem.unparent ();
122+
123+ //build the appmenu
124+ var menu = new Gtk.Menu ();
125+ menu.append (crop_menuitem);
126+ menu.append (preferences_menuitem);
127+ menu.append (new Gtk.SeparatorMenuItem ());
128+ menu.append (content_menuitem);
129+ menu.append (about_menuitem);
130+
131+ var appmenu = new Granite.Widgets.ToolButtonWithMenu.from_stock (
132+ Gtk.Stock.PROPERTIES, Gtk.IconSize.MENU, _("Menu"), menu);
133+ var appmenu_toolitem = new Gtk.ToolItem ();
134+ appmenu_toolitem.add (appmenu);
135+
136+ //create a spacer to align the elementary-menuitems on the right side
137+ var spacer = new Gtk.ToolItem ();
138+ spacer.set_expand (true);
139+
140+ var toolbar = (Gtk.Toolbar) builder.get_object ("toolbar1");
141+ toolbar.insert (spacer, 8);
142+ toolbar.insert (appmenu_toolitem, 9);
143+ toolbar.show_all ();
144+ }
145+
146 authorize_dialog = (Gtk.Dialog) builder.get_object ("authorize_dialog");
147 authorize_label = (Gtk.Label) builder.get_object ("authorize_label");
148 username_entry = (Gtk.Entry) builder.get_object ("username_entry");

Subscribers

People subscribed via source and target branches