After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 742853 - Big spinners in network panel
Big spinners in network panel
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Network
3.14.x
Other Linux
: Normal normal
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-01-13 13:39 UTC by Timm Bäder
Modified: 2015-01-20 19:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
wifi: Use a GtkStack to switch between button/spinner (6.04 KB, patch)
2015-01-13 13:48 UTC, Timm Bäder
none Details | Review
wifi: Use a GtkStack to switch between button/spinner (6.35 KB, patch)
2015-01-16 16:08 UTC, Timm Bäder
needs-work Details | Review
Screenshot, old on top, new on bottom (8.96 KB, image/png)
2015-01-16 16:08 UTC, Timm Bäder
  Details
wifi: Use a GtkStack to switch between button/spinner (6.29 KB, patch)
2015-01-16 18:09 UTC, Timm Bäder
needs-work Details | Review
wifi: Use a GtkStack to switch between button/spinner (6.38 KB, patch)
2015-01-20 18:03 UTC, Timm Bäder
needs-work Details | Review
wifi: Use a GtkStack to switch between button/spinner (6.39 KB, patch)
2015-01-20 19:20 UTC, Bastien Nocera
committed Details | Review

Description Timm Bäder 2015-01-13 13:39:21 UTC
Something that annoys me whenever I connect to a wifi is the big spinner I see in the row during the connection process. This is caused by the spinner and the button being in a size group, which is (I guess) the case so the spinner is really centered with regard to the button(s).

I'll attach a patch that instead of using a size group, uses a GtkStack, so the spinner can be centered without being the same size as the button.
Comment 1 Timm Bäder 2015-01-13 13:48:43 UTC
Created attachment 294431 [details] [review]
wifi: Use a GtkStack to switch between button/spinner

Instead of only adding a button and/or spinner when constructing the
row, always add both to a GtkStack and only show that stack when
necessary. This also removes the need for a GtkSizeGroup and the big
spinner caused by it.
Comment 2 Timm Bäder 2015-01-16 16:08:09 UTC
Created attachment 294699 [details] [review]
wifi: Use a GtkStack to switch between button/spinner

Instead of only adding a button and/or spinner when constructing the
row, always add both to a GtkStack and only show that stack when
necessary. This also removes the need for a GtkSizeGroup and the big
spinner caused by it.
Comment 3 Timm Bäder 2015-01-16 16:08:53 UTC
Created attachment 294700 [details]
Screenshot, old on top, new on bottom
Comment 4 Bastien Nocera 2015-01-16 16:36:21 UTC
Review of attachment 294699 [details] [review]:

Wouldn't it be easier to create a GtkStack with 3 widgets (spinner, button, and a placeholder), and switch between those 3, rather than showing/hiding the stack, and switching pages?

::: panels/network/net-device-wifi.c
@@ +1585,2 @@
         widget = NULL;
+        GtkWidget *image;

Declarations should happen at the top of the function.
Comment 5 Timm Bäder 2015-01-16 18:09:21 UTC
Created attachment 294704 [details] [review]
wifi: Use a GtkStack to switch between button/spinner

Instead of only adding a button and/or spinner when constructing the
row, always add both to a GtkStack and only show that stack when
necessary. This also removes the need for a GtkSizeGroup and the big
spinner caused by it.
Comment 6 Timm Bäder 2015-01-16 18:10:32 UTC
(In reply to comment #4)
> Wouldn't it be easier to create a GtkStack with 3 widgets (spinner, button, and
> a placeholder), and switch between those 3, rather than showing/hiding the
> stack, and switching pages?

Yes, that works too. I'm generally not a fan of abusing a label for that, but it *is* slightly less code (and state) in that case.
Comment 7 Bastien Nocera 2015-01-19 11:21:37 UTC
Review of attachment 294704 [details] [review]:

On the right path, but a couple of niggles.

::: panels/network/net-device-wifi.c
@@ +1556,3 @@
+
+        widget = gtk_label_new ("");
+        gtk_container_add (GTK_CONTAINER (button_stack), widget);

I prefer gtk_stack_add_named(), so you can avoid having to keep the widget around.

@@ +1597,3 @@
+        gtk_widget_set_valign (widget, GTK_ALIGN_CENTER);
+        atk_object_set_name (gtk_widget_get_accessible (widget), _("Options…"));
+        gtk_container_add (GTK_CONTAINER (button_stack), widget);

Ditto.

@@ +1926,3 @@
         spinner = GTK_WIDGET (g_object_get_data (G_OBJECT (row), "spinner"));
         edit = GTK_WIDGET (g_object_get_data (G_OBJECT (row), "edit"));
+        stack = GTK_WIDGET (g_object_get_data (G_OBJECT (row), "button_stack"));

Used "named" would avoid this.
Comment 8 Timm Bäder 2015-01-20 18:03:25 UTC
Created attachment 295038 [details] [review]
wifi: Use a GtkStack to switch between button/spinner

Instead of only adding a button and/or spinner when constructing the
row, always add both to a GtkStack and only show that stack when
necessary. This also removes the need for a GtkSizeGroup and the big
spinner caused by it.
Comment 9 Bastien Nocera 2015-01-20 19:18:39 UTC
Review of attachment 295038 [details] [review]:

Rest looks fine.

::: panels/network/net-device-wifi.c
@@ +1586,3 @@
         gtk_box_pack_start (GTK_BOX (row_box), gtk_label_new (""), TRUE, FALSE, 0);
 
+

Extra linefeed here.

@@ +1602,1 @@
         if (connection) {

No need for braces on one-line conditionals.

@@ +1618,3 @@
         gtk_widget_set_valign (widget, GTK_ALIGN_CENTER);
+        gtk_stack_add_named (GTK_STACK (button_stack), widget, "spinner");
+        if (connecting) {

No need for braces on one-line conditionals.
Comment 10 Bastien Nocera 2015-01-20 19:20:03 UTC
Created attachment 295042 [details] [review]
wifi: Use a GtkStack to switch between button/spinner

Instead of only adding a button and/or spinner when constructing the
row, always add both to a GtkStack and only show that stack when
necessary. This also removes the need for a GtkSizeGroup and the big
spinner caused by it.
Comment 11 Bastien Nocera 2015-01-20 19:21:06 UTC
Attachment 295042 [details] pushed as 9ffaff7 - wifi: Use a GtkStack to switch between button/spinner