GNOME Bugzilla – Bug 673869
Upstream some ubuntu design changes
Last modified: 2012-05-21 17:10:27 UTC
Some of the changes Ubuntu has made to the overview are pretty nice. Namely: * Increasing the icon size to the more standard GNOME 3 size of 48px * Making the window size a bit bigger by default and a bit smarter about small screens * Using separators between sections to help distinguish the grids of icons
Created attachment 211793 [details] [review] Increase icon size to 48px
Created attachment 211794 [details] [review] Increase default window size And be smarter about small screen sizes. Based on patch from Ubuntu
Created attachment 211795 [details] [review] Add horizontal separators between sections Based on patch from Ubuntu
Created attachment 211796 [details] [review] Fit three columns of search results And center labels vertically that have no search matching text.
Created attachment 211828 [details] screenshot
Review of attachment 211794 [details] [review]: You should be using the startup notification data to know where from the application was launched, not using gdk_device_manager_get_client_pointer(). This also means that: - moving the shell from a big screen to a small one won't work (external monitor to internal one) - lowering the resolution of the display won't change the shell's size All that to basically paper over the fact that there are bug in GtkIconView which make it impossible to know the realized height of the full scrolled view.
Review of attachment 211793 [details] [review]: Looks good.
Review of attachment 211795 [details] [review]: Looks fine.
Review of attachment 211796 [details] [review]: Looks good too. Could you mention the new default shell width from the other patch, or better, make this new width a calculation based on that new width?
Created attachment 211955 [details] [review] Hide window titlebar when maximized
*** Bug 666000 has been marked as a duplicate of this bug. ***
Created attachment 213154 [details] [review] Increase default window size And be smarter about small screen sizes. Based on patch from Ubuntu
Created attachment 213155 [details] [review] Add horizontal separators between sections Based on patch from Ubuntu
Created attachment 213156 [details] [review] Fit three columns of search results And center labels vertically that have no search matching text.
Created attachment 213157 [details] [review] Hide window titlebar when maximized
*** Bug 654977 has been marked as a duplicate of this bug. ***
*** Bug 657560 has been marked as a duplicate of this bug. ***
*** Bug 645461 has been marked as a duplicate of this bug. ***
Duped some bugs here which cover some of the same aspects
Created attachment 213220 [details] [review] Increase default window size And be smarter about small screen sizes. Based on patch from Ubuntu
Created attachment 213221 [details] [review] Add horizontal separators between sections Based on patch from Ubuntu
Created attachment 213222 [details] [review] Fit three columns of search results And center labels vertically that have no search matching text.
Created attachment 213223 [details] [review] Hide window titlebar when maximized
Committed a slightly reworked version.
Review of attachment 213223 [details] [review]: ::: shell/gnome-control-center.c @@ +245,3 @@ gtk_window_set_icon_name (GTK_WINDOW (priv->window), priv->default_window_icon); + gtk_window_set_hide_titlebar_when_maximized (GTK_WINDOW (priv->window), TRUE); Why is this in shell_show_overview_page() and not gnome_control_center_init()? Consider the following case: 1) Open a specific panel from one of the shell menus (e.g. "Power Settings") => titlebar 2) Switch to overview => no titlebar 3) Switch back to the panel from 1) => no titlebar
I've tried these patches - in general they work nicely, but I still get a scrollbar even though the window is not full height. Maybe that is because I am using 3.4 desktop files, and still have a 3-line "Wacom Graphics Tablet" ?
Review of attachment 213222 [details] [review]: ::: shell/shell-search-renderer.c @@ +216,3 @@ shell_search_renderer_set_layout (SHELL_SEARCH_RENDERER (cell), widget); + pango_layout_set_width (priv->layout, PANGO_SCALE * 164); There's still a magic number here.
Review of attachment 213220 [details] [review]: ::: shell/gnome-control-center.c @@ +774,3 @@ + gtk_widget_set_margin_bottom (shell->priv->main_vbox, 8); + gtk_widget_set_margin_left (shell->priv->main_vbox, 12); + gtk_widget_set_margin_right (shell->priv->main_vbox, 12); We shouldn't really have margin changes in this patch. @@ +1254,3 @@ priv->default_window_icon = g_strdup (gtk_window_get_icon_name (GTK_WINDOW (priv->window))); + gtk_widget_hide (W (priv->builder, "home-button")); I don't want that change. The notebook_switch_page_cb() function is the one that's supposed to hide the various elements depending on the notebook page. ::: shell/shell.ui @@ +5,3 @@ <object class="GtkWindow" id="main-window"> <property name="title" translatable="yes">System Settings</property> + <property name="resizable">True</property> Do we really need to change that? update_small_screen_settings()'s code should be enough.
Review of attachment 213221 [details] [review]: Looks good.
*** Bug 651572 has been marked as a duplicate of this bug. ***
(In reply to comment #26) > I've tried these patches - in general they work nicely, but I still get a > scrollbar even though the window is not full height. Maybe that is because I am > using 3.4 desktop files, and still have a 3-line "Wacom Graphics Tablet" ? No. It's because GtkIconView's sizing is broken. It will tell you its natural height is -1, not taking into account the height of any of its children (icons + labels).
(In reply to comment #31) > (In reply to comment #26) > > I've tried these patches - in general they work nicely, but I still get a > > scrollbar even though the window is not full height. Maybe that is because I am > > using 3.4 desktop files, and still have a 3-line "Wacom Graphics Tablet" ? > > No. It's because GtkIconView's sizing is broken. It will tell you its natural > height is -1, not taking into account the height of any of its children (icons > + labels). Filed bug 675290 about it.
Created attachment 213631 [details] [review] Increase icon size to 48px
Created attachment 213632 [details] [review] Increase default window size And be smarter about small screen sizes. Based on patch from Ubuntu
Created attachment 213633 [details] [review] Add horizontal separators between sections Based on patch from Ubuntu
Created attachment 213634 [details] [review] Fit three columns of search results And center labels vertically that have no search matching text.
Created attachment 213635 [details] [review] Hide window titlebar when maximized
Created attachment 213636 [details] [review] Use menubar style class for toolbar
Still doesn't maximize properly because of bug 675242.
Comment on attachment 213632 [details] [review] Increase default window size Split into 2, made some changes related to sizing, as I've cleaned this up following GTK+ bug fixes in GtkIconView. Also added a FIXME.
Attachment 213631 [details] pushed as 599f19a - Increase icon size to 48px Attachment 213633 [details] pushed as 620e701 - Add horizontal separators between sections Attachment 213634 [details] pushed as 796d4eb - Fit three columns of search results Attachment 213635 [details] pushed as 9c2e8c2 - Hide window titlebar when maximized
Review of attachment 213636 [details] [review]: Bizarrely, this seems to add a 1 pixel border on each side of the scrolled window, which causes it to not be able to fit 6 items per row, which causes it to not have the correct height to fit all the items without a scrollbar. This small hack fixes it, but I'd like Cosimo to look at the best way we can fix this. diff --git a/shell/gnome-control-center.c b/shell/gnome-control-center.c index 412a20e..78c9231 100644 --- a/shell/gnome-control-center.c +++ b/shell/gnome-control-center.c @@ -839,7 +839,7 @@ notebook_switch_page_cb (GtkNotebook *book, gtk_widget_get_preferred_height_for_width (GTK_WIDGET (priv->main_vbox), FIXED_WIDTH, NULL, &nat_height); gtk_widget_set_size_request (priv->scrolled_window, FIXED_WIDTH, - priv->small_screen ? SMALL_SCREEN_FIXED_HEIGHT : nat_height); + priv->small_screen ? SMALL_SCREEN_FIXED_HEIGHT : nat_height + 2); } else {
Cosimo, is there a way to know how much space shadows will be taking when we do: gtk_widget_get_preferred_height_for_width (GTK_WIDGET (priv->main_vbox), FIXED_WIDTH, NULL, &nat_height); gtk_widget_set_size_request (priv->scrolled_window, FIXED_WIDTH, nat_height); (main_vbox is inside the scrolled_window).
Unfortunately, the reworked patch doesn't allow maximizing the window in small mode.
Attachment 213636 [details] pushed as 3bc7576 - Use menubar style class for toolbar
Reopening. There's still a bug left with the maximisation not working, probably because of a race between gtk_window_set_resizable() and gtk_window_maximize().
Maximise not working 1. Big screen 2. xrandr to small screen size 3. small screen mode but we're not maximised because of the bug in comment 46 Migrating from small to big display doesn't work anymore Window size when unmaximising: 1. Small screen 2. Drag off the panel 3. Size should be 80% of workarea And the FIXME in the main code.
(In reply to comment #47) > Maximise not working > 1. Big screen > 2. xrandr to small screen size > 3. small screen mode but we're not maximised because of the bug in comment 46 Filed bug 675874 with test case.
(In reply to comment #47) > Migrating from small to big display doesn't work anymore This should be fixed, unfortunately, bug 675874 really gets in the way. > Window size when unmaximising: > 1. Small screen > 2. Drag off the panel > 3. Size should be 80% of workarea This seems to work alright for me, not sure whether we want a different behaviour here. Would need exact reproducer (with the size used for example). The 80% value comes from: http://git.gnome.org/browse/mutter/tree/src/core/window.c#n59 > And the FIXME in the main code. Another race problem, I wanted to use gdk_screen_get_monitor_workarea() but the window manager won't have updated the values by the time we process the signal.