GNOME Bugzilla – Bug 692174
Use a GNOME 3 style titlebar/toolbar
Last modified: 2013-02-18 15:42:04 UTC
I think we should use a GNOME 3 style titlebar for the panels. * Always use Settings for the window title * Put the panel title in the center position of the titlebar/toolbar * Remove the "Enabled" label from the Search panel switch * Remove the Bluetooth label from the bluetooth panel switch * etc
*** Bug 692304 has been marked as a duplicate of this bug. ***
Created attachment 236458 [details] [review] Refactor the shell
Created attachment 236459 [details] [review] Make CcShell an interface on an instance of an application window
Created attachment 236460 [details] [review] Rename shell window to CcWindow
Created attachment 236461 [details] [review] Don't use gtkbuilder for constructing the shell
Created attachment 236462 [details] [review] Use GdHeaderBar in the shell
Created attachment 236463 [details] [review] shell: put the panel title in the header instead of the window title
Created attachment 236482 [details] [review] Use GdHeaderBar in the shell
Created attachment 236483 [details] [review] shell: put the panel title in the header instead of the window title
*** Bug 676557 has been marked as a duplicate of this bug. ***
Review of attachment 236458 [details] [review]: Missing "shell: " prefix to the commit subject. The cc-application.h/control-center.c pairing is bad, rename control-center.c It would be good to explain in the commit message *why* you're doing this. Looks like shuffling code around from this end. ::: shell/control-center.c @@ +186,2 @@ if (start_panels[1]) + g_debug ("Extra argument: %s", start_panels[1]); Why are those lines changed? @@ +188,2 @@ else + g_debug ("No extra argument"); I don't see any differences. Spaces cleanups? @@ +226,3 @@ } + Line feeds.
Review of attachment 236459 [details] [review]: Missing "shell: " prefix in subject line. I don't understand why those changes are needed again. ::: shell/cc-shell.h @@ +62,3 @@ GType cc_shell_get_type (void) G_GNUC_CONST; +CcPanel * cc_shell_get_active_panel (CcShell *shell); Indentation fixes can go in a separate patch. ::: shell/gnome-control-center.c @@ +59,3 @@ #define MIN_ICON_VIEW_HEIGHT 300 +#define DEFAULT_WINDOW_TITLE _("Settings") That should be: N_("Settings"); and you'd call _(DEFAULT_WINDOW_TITLE) when used. @@ +264,3 @@ /* reset window title and icon */ + gtk_window_set_role (GTK_WINDOW (self), NULL); + gtk_window_set_title (GTK_WINDOW (self), DEFAULT_WINDOW_TITLE); Changing how the window defaults are set should be a separate patch. @@ -1352,3 @@ -#ifdef HAVE_CHEESE - if (gtk_clutter_init (NULL, NULL) != CLUTTER_INIT_SUCCESS) And that just disappears?
Review of attachment 236460 [details] [review]: Missing prefix on subject line. Again, why? ::: shell/gnome-control-center.c @@ +1363,2 @@ str = g_strdup_printf ("%u", (guint) GDK_WINDOW_XID (window)); + g_setenv ("CC_WINDOW_XID", str, TRUE); Excessive sed job.
Review of attachment 236461 [details] [review]: Why?
Review of attachment 236482 [details] [review]: Looks fine.
Review of attachment 236483 [details] [review]: Looks fine.
Review of attachment 236458 [details] [review]: I will fix the commit comment but I think the other comments are because splinter isn't understanding that I did in fact rename control-center.c to cc-application.c. ::: shell/control-center.c @@ +188,2 @@ else + g_debug ("No extra argument"); I think splinter has a bug. These really aren't changed. The file was renamed from control-center.c to cc-application.c. I guess it confused it.
Review of attachment 236459 [details] [review]: ::: shell/gnome-control-center.c @@ +59,3 @@ #define MIN_ICON_VIEW_HEIGHT 300 +#define DEFAULT_WINDOW_TITLE _("Settings") Oops. @@ +264,3 @@ /* reset window title and icon */ + gtk_window_set_role (GTK_WINDOW (self), NULL); + gtk_window_set_title (GTK_WINDOW (self), DEFAULT_WINDOW_TITLE); It isn't easily separable. The reason it set the title in the way it did was because it was trying to pull the default title out of the child window from the uibuilder. Once we make the class a window itself we no longer need this. @@ -1352,3 @@ -#ifdef HAVE_CHEESE - if (gtk_clutter_init (NULL, NULL) != CLUTTER_INIT_SUCCESS) It was being done in two places for some reason. It is removed from here but left in the application initialization where it probably belongs.
Created attachment 236592 [details] [review] shell: make the application a GtkApplication subclass This promotes better encapsulation and allows us to move application logic out of main() and rename the confusingly named control-center.c to main.c
Created attachment 236593 [details] [review] shell: make the main app window a subclass of GtkApplicationWindow This allows for better encapsulation of window logic. In order to do this CcShell was made an interface instead of an abstract base class. Which makes much more sense anyway.
Created attachment 236595 [details] [review] shell: rename shell window to CcWindow This avoids the confusion of the name gnome-control-center.c and is consistent with all the other classes/files in the project.
Created attachment 236596 [details] [review] shell: don't use gtkbuilder for constructing the shell So that we can more easily add GdHeaderBar in the following patches. Much of the advantage of using gtkbuilder was lost anyway because we couldn't really use it from glade without corrupting the file.