GNOME Bugzilla – Bug 672070
Gradient system theme background looks wrong on vertical panels
Last modified: 2012-03-14 17:12:38 UTC
Created attachment 209729 [details] Vertical panel [with Radiance theme] Because there is no mechanism to change the system theme background based on the orientation of the panel, themed vertical panels look wrong. Attached is a screenshot of a vertical panel themed with Radiance.
Created attachment 209730 [details] [review] [Patch 1/1] Add style class for orientation to menubar Patch series for adding "horizontal" or "vertical" style classes to panel containers
Created attachment 209731 [details] [review] [Patch 2/5] Add style class for orientation to PanelAppletFrame
Created attachment 209732 [details] [review] [Patch 3/5] Add style class for orientation to PanelWidget
Created attachment 209734 [details] [review] [Patch 4/5] Add style class for orientation to PanelApplet
Created attachment 209736 [details] [review] [Patch 5/5] Add style class for orientation to PanelToplevel
Created attachment 209740 [details] Screenshot of vertical panel [after patches] Naturally, changes to the system theme were necessary to reflect the new style classes.
Review of attachment 209730 [details] [review]: Can you make sure to add a "panel: " prefix to the first line in commit logs? ::: gnome-panel/panel-menu-bar-object.c @@ +69,3 @@ GTK_STYLE_PROVIDER_PRIORITY_APPLICATION); g_object_unref (provider); gtk_style_context_add_class (context, "gnome-panel-menu-bar"); Add an empty line here, please. @@ +344,3 @@ + gtk_style_context_add_class (context, GTK_STYLE_CLASS_VERTICAL); + gtk_style_context_remove_class (context, GTK_STYLE_CLASS_HORIZONTAL); + } Missing call to gtk_widget_reset_style()?
Review of attachment 209732 [details] [review]: ::: gnome-panel/panel-widget.c @@ +2540,3 @@ + GtkStyleContext *context; + + g_return_if_fail (PANEL_IS_WIDGET (panel_widget)); This test is not needed, really :-)
Review of attachment 209734 [details] [review]: ::: libpanel-applet/panel-applet.c @@ +679,1 @@ applet->priv->orient = orient; I'd prefer to do "applet->priv->orient = orient;" before your changes. It just feels a bit more natural.
Review of attachment 209734 [details] [review]: ::: libpanel-applet/panel-applet.c @@ +2006,3 @@ + context = gtk_widget_get_style_context (GTK_WIDGET (applet)); + gtk_style_context_add_class (context, GTK_STYLE_CLASS_HORIZONTAL); Forgot to say: I'd prefer to move this just before the gtk_container_add() at the end of the function.
Review of attachment 209736 [details] [review]: ::: gnome-panel/panel-toplevel.c @@ +4341,3 @@ + context = gtk_widget_get_style_context (GTK_WIDGET (toplevel)); + gtk_style_context_add_class (context, GTK_STYLE_CLASS_HORIZONTAL); Please move this at the end of the function. @@ +4818,3 @@ } toplevel->priv->orientation = orientation; Please add an empty line here.
Created attachment 209752 [details] [review] [PATCH v2 1/5] panel: Add style class for orientation to menubar Patch series v2
Created attachment 209753 [details] [review] [PATCH v2 2/5] panel: Add style class for orientation to PanelAppletFrame
Created attachment 209754 [details] [review] [PATCH v2 3/5] panel: Add style class for orientation to PanelWidget
Created attachment 209755 [details] [review] [PATCH v2 4/5] libpanel-applet: Add style class for orientation to PanelApplet
Created attachment 209756 [details] [review] [PATCH v2 5/5] panel: Add style class for orientation to PanelToplevel
Thanks, I've pushed all patches.