GNOME Bugzilla – Bug 703974
Fix the alignment of the switch buttons
Last modified: 2013-07-30 12:34:15 UTC
See patches.
Created attachment 248897 [details] [review] bluetooth: fix the alignment of the switch button
Created attachment 248898 [details] [review] search: fix the alignment of the switch button
Created attachment 248899 [details] [review] sharing: fix the alignment of the switch button
Created attachment 248900 [details] Screenshot - before
Created attachment 248901 [details] Screenshot - after
Review of attachment 248897 [details] [review]: Nice catch. I think it is pretty clear that the switch was not indented to be vertically stretched. However, the fix results in an asymmetric padding around the switch. More on the bottom than on the right. I wonder if we should compensate for that with a right margin. ::: panels/bluetooth/cc-bluetooth-panel.c @@ +813,3 @@ + switch_button = WID ("switch_bluetooth"); + gtk_widget_set_valign (switch_button, GTK_ALIGN_CENTER); + I suspect it is cleaner to make this change in the UI file.
Thanks again for the patches! Just a general comment, the commit message in the patches should describe the problem a bit more. Doesn't have to be a book but at least one sentence would be nice.
Created attachment 248976 [details] [review] bluetooth: fix the alignment of the switch button
Review of attachment 248976 [details] [review]: As Jon mentioned though, the commit message isn't good. The subject should describe the problem fixed, so it could be "Fix stretched header bar switch". The right padding problem is also a blocker. ::: panels/bluetooth/bluetooth.ui @@ +9,3 @@ <property name="visible">True</property> <property name="can_focus">True</property> + <property name="valign">3</property> you can use the symbolic name here, so it should be "center".
Review of attachment 248898 [details] [review]: Same comments as for the Bluetooth panel changes. ::: panels/search/cc-search-panel.c @@ +614,2 @@ widget = gtk_switch_new (); + gtk_widget_set_valign (widget, GTK_ALIGN_CENTER); Needs to be in the .ui file instead.
Review of attachment 248899 [details] [review]: ::: panels/sharing/cc-sharing-panel.c @@ +1015,3 @@ /* create the master switch */ priv->master_switch = gtk_switch_new (); + gtk_widget_set_valign (priv->master_switch, GTK_ALIGN_CENTER); Ditto.
*** Bug 698514 has been marked as a duplicate of this bug. ***
Created attachment 249080 [details] [review] bluetooth: Fix stretched header bar switch it is also adds padding of 4px.
Created attachment 249081 [details] [review] search: Fix stretched header bar switch (In reply to comment #10) > Review of attachment 248898 [details] [review]: > > Same comments as for the Bluetooth panel changes. > > ::: panels/search/cc-search-panel.c > @@ +614,2 @@ > widget = gtk_switch_new (); > + gtk_widget_set_valign (widget, GTK_ALIGN_CENTER); > > Needs to be in the .ui file instead. How? The switch button is defined in the code, not in the .ui file.
Created attachment 249082 [details] [review] sharing: Fix stretched header bar switch (In reply to comment #11) > Review of attachment 248899 [details] [review]: > > ::: panels/sharing/cc-sharing-panel.c > @@ +1015,3 @@ > /* create the master switch */ > priv->master_switch = gtk_switch_new (); > + gtk_widget_set_valign (priv->master_switch, GTK_ALIGN_CENTER); > > Ditto. Ditto: the switch button is defined in the code, not in the .ui file.
Created attachment 249084 [details] After - with padding
*** Bug 702363 has been marked as a duplicate of this bug. ***
(In reply to comment #16) > Created an attachment (id=249084) [details] > After - with padding Looks good.
Review of attachment 249080 [details] [review]: Looks good
Review of attachment 249081 [details] [review]: Looks good
Review of attachment 249082 [details] [review]: Looks good
Review of attachment 249080 [details] [review]: pushed as ff8a214c5f793419b5b8fae47043fabd10dee202 - bluetooth: Fix stretched header bar switch
Review of attachment 249081 [details] [review]: pushed as 2ebf5e7dd3474cd827fa6052faac73af893f119d - search: Fix stretched header bar switch
Review of attachment 249082 [details] [review]: pushed as dd1a0c8dce17f3f3c993331d54320a304ca03772 - sharing: Fix stretched header bar switch
Missing patch for network panel (airplane mode switch).
Created attachment 250335 [details] [review] network: Fix stretched header bar switch
Review of attachment 250335 [details] [review]: Looks good.
Review of attachment 250335 [details] [review]: pushed as 30e0080ccc1f594850bf17120cf360d758b1e9b2 - network: Fix stretched header bar switch