GNOME Bugzilla – Bug 666103
Should be able to match the parent where the a dropdown menu is child
Last modified: 2012-01-12 19:04:21 UTC
In the css theme you should be able to match various dropdown menus to apply different styles. For example, you have a dark entry and it's dropdown menu should be dark, but the combobox is light and you want a light dropdown.
Created attachment 203569 [details] [review] First approach
Created attachment 203574 [details] [review] Also update on style-updated
I don't think the second patch makes sense for now.
Created attachment 204745 [details] [review] patch for the gtk dir
Created attachment 204746 [details] [review] patch for the docs dir
Created attachment 204747 [details] [review] patch for the docs dir
Review of attachment 204745 [details] [review]: Whats the relationship between this attach-to widget and transient-for ? That should be spelled out in the docs.
Review of attachment 204745 [details] [review]: Looks generally good to me, if our APIs overlords (Benjamin/Matthias) are fine with the new function names; some comments below. ::: gtk-old/gtkwidget.c @@ +8310,3 @@ + + if (attached_windows) + g_list_foreach (attached_windows, attached_windows_gfunc, NULL); Can't you use ((GFunc) reset_style_recurse) here instead of attached_window_gfunc? @@ +13805,3 @@ + + widget->priv->path = gtk_widget_path_copy (gtk_widget_get_path (attach_widget)); + } I think this would be more readable: GtkWidget *attach_widget = NULL; if (GTK_IS_WINDOW (widget)) attach_widget = gtk_window_get_attached_to (GTK_WINDOW (widget)) if (attach_widget != NULL) widget->priv->path = gtk_widget_path_copy (gtk_widget_get_path (attach_widget)); else widget->priv->path = gtk_widget_path_new (); ::: gtk-old/gtkwindow.c @@ +982,3 @@ + * GtkWindow:attached-to: + * + * the widget attached to the window. Add a link to gtk_window_set_attached_to() here. @@ +2440,3 @@ + GList* attached_windows_new = g_list_remove (attached_windows, window); + g_object_set_data (G_OBJECT (priv->attach_widget), "gtk-attached-windows", attached_windows_new); + } Is there a reason why you use g_object_get/set_data here? I think it would be cleaner to have GList *attached_windows as a private member of GtkWidget and having two private methods, _gtk_widget_get/set_attached_windows to access it. @@ +2652,3 @@ + * @attach_widget (allow-none): a #GtkWidget, or %NULL + * + * Attach the window to a widget. This definitely needs a more detailed description (e.g. in which cases are clients supposed to call this API?). It should also be made clear that this refers only to the parsing of style information and has nothing to do with gtk_window_set_transient_for().
Thx for the detailed review. Yes, I'm fine with the API. (In reply to comment #8) > It should also be made clear that this refers only to the parsing of style > information and has nothing to do with gtk_window_set_transient_for(). > I envisioned this not just meant for parsing of style information. An immediate use case is at least the accessibility code that wants to report the attach widget as the parent of the popup. (It currently does that manually in a few cases I think). In general I envision this API as a way for widgets to say "this window belongs to me", so I expect this to be used by tooltips, entry completions, right-click popups, ctrl-f search entries and whatnot. I'd ideally want to make these windows real child widgets, but I'm afraid that would break too much. In general, every popup window spawned by GTK should have this set.
Created attachment 205049 [details] [review] patch for the gtk dir, updated
Review of attachment 205049 [details] [review]: Looks good to me with the following nitpick. ::: gtk-old/gtkwidget.c @@ +13725,3 @@ +{ + return widget->priv->attached_windows; +} This is not used, so please remove it. ::: gtk-old/gtkwidgetprivate.h @@ +105,3 @@ +void _gtk_widget_remove_attached_window (GtkWidget *widget, + GtkWindow *window); +GList *_gtk_widget_get_attached_windows (GtkWidget *widget); Ditto.
Review of attachment 205049 [details] [review]: A few minor comments... ::: gtk-old/gtkwidget.c @@ +8303,3 @@ reset_style_recurse (widget, NULL); + + if (widget->priv->attached_windows) no need for the if, NULL is a valid empty list @@ +13710,3 @@ + GtkWindow *window); +{ + widget->priv->attached_windows = g_list_append (widget->priv->attached_windows, window); does order matter? I do not think so, so better to prepend @@ +13717,3 @@ + GtkWindow *window); +{ + if (widget->priv->attached_windows) no need for the if ::: gtk-old/gtkwindow.c @@ -2408,0 +2431,10 @@ +remove_attach_widget (GtkWindow *window) +{ + GtkWindowPrivate *priv = window->priv; ... 7 more ... g_object_clear?
Created attachment 205051 [details] [review] patch for the gtk dir, updated second time Also fixes a couple of typos in the code (; after the functions)
Would be nice to unify this new api with the existing gtk_menu_attach_to_widget
One thing at a time :-) I would probably be able to use the api in other widgets, so if someone could create a new bugreport I could look into it...
Pushed this to master now, thanks Andrea and everyone else involved!