GNOME Bugzilla – Bug 694325
Use GtkStack instead of notebooks
Last modified: 2021-06-09 16:06:18 UTC
So we can do nice resizes and stuff.
Created attachment 237040 [details] [review] shell: Use GdStack for switching panels
This was a bug in gd-header-bar, fixed in libgd master.
Pushed to gnome-3-8 and master Attachment 237040 [details] pushed as 575bd62 - shell: Use GdStack for switching panels
Reopening, there's other places where we should be using GdStack.
in 3.10, you should even be using GtkStack
(In reply to comment #5) > in 3.10, you should even be using GtkStack We are now. I've retitled the bug to match the new purpose of the bug. We would need clearer guidelines as to what parts of the UI should have a transition when switching content.
Created attachment 277955 [details] [review] cc-editable-entry: use GtkStack instead of GtkNotebook It also fixes following warnings: (gnome-control-center:32638): Gtk-WARNING **: GtkEntry 0x28bcef0 is mapped but not child_visible (gnome-control-center:32638): Gtk-WARNING **: GtkEntry 0x28bcef0 is mapped but visible=1 child_visible=0 parent GtkNotebook 0x2b86600 mapped=1
Review of attachment 277955 [details] [review]: ::: shell/cc-editable-entry.c @@ +610,3 @@ /* Entry */ priv->entry = (GtkEntry*)gtk_entry_new (); + gtk_container_add (GTK_CONTAINER (priv->stack), GTK_WIDGET (priv->entry)); I'd rather you used gtk_stack_add_named(), and used gtk_stack_set_visible_child_name() to switch between stack children.
Created attachment 278340 [details] [review] cc-editable-entry: use GtkStack instead of GtkNotebook Thanks for the review. Named children are used now. In additional the mechanism of setting label padding had to be changed, because noneditable widget has wrong padding (size-allocate signal was not called on button).
Created attachment 278341 [details] [review] um-editable-combo: use GtkStack instead of GtkNotebook Also similar patch for um-editable-combo: The mechanism of setting label padding had to be changed consequently. It fixes following warnings: (gnome-control-center:32638): Gtk-WARNING **: GtkComboBox 0x28bcef0 is mapped but not child_visible (gnome-control-center:32638): Gtk-WARNING **: GtkComboBoy 0x28bcef0 is mapped but visible=1 child_visible=0 parent GtkNotebook 0x2b86600 mapped=1 The ugly hack to catch when combo is losing focus had to be also removed and notify::popup-shown is used instead. It fixes when GtkComboBox is wrongly shown instead of GtkButton due to escape is pressed or active item is selected.
Review of attachment 278340 [details] [review]: ::: shell/cc-editable-entry.c @@ +514,3 @@ e->priv->in_stop_editing = TRUE; + gtk_stack_set_visible_child_name (e->priv->stack, PAGE_BUTTON); + gtk_widget_grab_focus (GTK_WIDGET (e->priv->button)); Is grabbing the focus necessary here? @@ +525,3 @@ gtk_entry_set_text (e->priv->entry, cc_editable_entry_get_text (e)); + gtk_stack_set_visible_child_name (e->priv->stack, PAGE_BUTTON); + gtk_widget_grab_focus (GTK_WIDGET (e->priv->button)); Ditto.
Review of attachment 278341 [details] [review]: Does using the keyboard to edit the drop-down still work? With the GtkNotebook you can: - tab to the button - press the button using space - press escape or arrows and enter to choose a new entry - it turns back into a button after tabbing away ::: panels/user-accounts/um-editable-combo.c @@ +307,3 @@ gtk_combo_box_get_active (combo->priv->combo)); + gtk_stack_set_visible_child_name (combo->priv->stack, PAGE_BUTTON); + gtk_widget_grab_focus (GTK_WIDGET (combo->priv->button)); Why do we need to grab the focus for the button again? @@ +318,3 @@ um_editable_combo_get_active (combo)); + gtk_stack_set_visible_child_name (combo->priv->stack, PAGE_BUTTON); + gtk_widget_grab_focus (GTK_WIDGET (combo->priv->button)); Ditto.
(In reply to comment #11) > Review of attachment 278340 [details] [review]: > > ::: shell/cc-editable-entry.c > @@ +514,3 @@ > e->priv->in_stop_editing = TRUE; > + gtk_stack_set_visible_child_name (e->priv->stack, PAGE_BUTTON); > + gtk_widget_grab_focus (GTK_WIDGET (e->priv->button)); > > Is grabbing the focus necessary here? Grabbing the focus is necessary to have same behavioral as before (as other widgets). The focus is lost when the GtkComboBox/GtkEntry is shown and escape/enter is pressed, but the GtkButton should have the focus instead. But I see the problem, with every gtk_combo_box_set_active_iter() etc. it grabs the focus which is wrong :-/ I can grab it only if gtk_widget_has_focus(), or do you have better idea how to fix it? (In reply to comment #12) > Review of attachment 278341 [details] [review]: > > Does using the keyboard to edit the drop-down still work? > With the GtkNotebook you can: > - tab to the button > - press the button using space > - press escape or arrows and enter to choose a new entry > - it turns back into a button after tabbing away Yes, it works.
(In reply to comment #13) > (In reply to comment #11) > > Review of attachment 278340 [details] [review] [details]: > > > > ::: shell/cc-editable-entry.c > > @@ +514,3 @@ > > e->priv->in_stop_editing = TRUE; > > + gtk_stack_set_visible_child_name (e->priv->stack, PAGE_BUTTON); > > + gtk_widget_grab_focus (GTK_WIDGET (e->priv->button)); > > > > Is grabbing the focus necessary here? > > Grabbing the focus is necessary to have same behavioral as before (as other > widgets). The focus is lost when the GtkComboBox/GtkEntry is shown and > escape/enter is pressed, but the GtkButton should have the focus instead. But I > see the problem, with every gtk_combo_box_set_active_iter() etc. it grabs the > focus which is wrong :-/ I can grab it only if gtk_widget_has_focus(), or do > you have better idea how to fix it? Adding a gtk_widget_has_focus() is fine by me. > (In reply to comment #12) > > Review of attachment 278341 [details] [review] [details]: > > > > Does using the keyboard to edit the drop-down still work? > > With the GtkNotebook you can: > > - tab to the button > > - press the button using space > > - press escape or arrows and enter to choose a new entry > > - it turns back into a button after tabbing away > > Yes, it works. Great.
Created attachment 279050 [details] [review] cc-editable-entry: use GtkStack instead of GtkNotebook Check gtk_widget_has_focus() before grabbing it.
Created attachment 279052 [details] [review] um-editable-combo: use GtkStack instead of GtkNotebook Check focus before grabbing using new function...
Review of attachment 279050 [details] [review]: Looks good.
Review of attachment 279052 [details] [review]: Looks good otherwise. ::: panels/user-accounts/um-editable-combo.c @@ +310,3 @@ + widget = gtk_window_get_focus (window); + if (widget) + { On the same line as the conditional.
Comment on attachment 279050 [details] [review] cc-editable-entry: use GtkStack instead of GtkNotebook commit 07f3416e043ffe49a6c74ff6619799d528bff8ae
Comment on attachment 279052 [details] [review] um-editable-combo: use GtkStack instead of GtkNotebook commit 58c1bbc2553b2998b6ac4eafe4e9f7bd2a699201
Thanks for the reviews :-)
Created attachment 283380 [details] [review] cc-editable-button: use GtkStack instead of GtkNotebook
Review of attachment 283380 [details] [review]: Looks good.
Comment on attachment 283380 [details] [review] cc-editable-button: use GtkStack instead of GtkNotebook Thanks for the review :-) commit ed4653627bcd065ea0bcdd559fe7e7b057e6e935
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new bug report at https://gitlab.gnome.org/GNOME/gnome-control-center/-/issues/ Thank you for your understanding and your help.