GNOME Bugzilla – Bug 724299
Use Header Bars
Last modified: 2014-04-29 10:22:26 UTC
Some patches to enable using header bars in dialogs.
patches still coming ?
Oh, I forgot this, I'll uplade a patches in a hour.
Created attachment 269247 [details] [review] notifications: Use header bar in edit dialog
Created attachment 269248 [details] [review] search: Use header bar in the location dialog
Created attachment 269249 [details] [review] background: Use header bar in the chooser dialog
Created attachment 269250 [details] [review] common: Use header bar in the language chooser dialog
Created attachment 269251 [details] [review] datetime: Use header bar in the datetime and the timezone dialogs
Created attachment 269252 [details] [review] display: Use header bar in the dialogs
Created attachment 269253 [details] [review] info: Use header bar in the extra options dialog
Created attachment 269254 [details] [review] keyboard: Use header bar in the custom shortcut dialog
Created attachment 269255 [details] [review] mouse: Use header bar in the test settings dialog
Created attachment 269256 [details] [review] power: Use header bar in the automatic suspend dialog
Created attachment 269257 [details] [review] privacy: Use header bar in all the dialogs
Created attachment 269258 [details] [review] region: Use header bar in all the dialogs
Created attachment 269259 [details] [review] sharing: Use header bar in all the dialogs
Created attachment 269260 [details] [review] universal-access: Use header bar in all the dialogs
Created attachment 269261 [details] [review] universal-access: Use header bar in all the dialogs
Created attachment 269262 [details] [review] user-accounts: Use header bar in the account dialog
Created attachment 269263 [details] [review] user-accounts: Use header bar in the password dialog
Created attachment 269264 [details] [review] user-accounts: Use header bar in the photo dialog
Created attachment 269265 [details] [review] sound: Use header bar in the testing dialog
Created attachment 269266 [details] [review] user-accounts: Use header bar in the history dialog
In general looks good. There are some bugs though: region and language - language and formats dialogs should be presenation dialogs (just a close button), not action dialogs background - stack switcher should be in the header bar, replacing the heading keyboard - custom shortcut dialog - button should read "Add", not "Apply" universal access - screen reader dialog - bottom of the dialog is cut off universal access - visual alerts dialog - has a "close" button on the left. please move "Test Flash" to the left and use a regular X close button users - login history - no close button is displayed. instead there is always a left and right arrow button users - fingerprint login - there are a few oddities here (cancel button next to back, missing border around the list) notifications - very tight padding around the content. needs to be at least 12px (this might not be a new bug - hard to tell)
Comment on attachment 269261 [details] [review] universal-access: Use header bar in all the dialogs The commit is wrong for this one.
Created attachment 270782 [details] [review] background: Use header bar in the chooser dialog
Created attachment 270783 [details] [review] keyboard: Use header bar in the custom shortcut dialog
Created attachment 270784 [details] [review] common: Use header bar in the language chooser dialog
Created attachment 270785 [details] [review] region: Use header bar in all the dialogs
Created attachment 270786 [details] [review] universal-access: Use header bar in all the dialogs
Review of attachment 269247 [details] [review]: ::: panels/notifications/cc-edit-dialog.c @@ +58,3 @@ shell = GTK_WINDOW (gtk_widget_get_toplevel (GTK_WIDGET (panel))); win = GTK_DIALOG (gtk_dialog_new ()); This line needs to go
Review of attachment 269252 [details] [review]: ::: panels/display/cc-display-panel.c @@ +1509,3 @@ gtk_window_set_modal (GTK_WINDOW (priv->dialog), TRUE); gtk_dialog_add_button (GTK_DIALOG (priv->dialog), _("_Cancel"), + GTK_RESPONSE_CANCEL); Unrelated change, drop it please. @@ +1952,3 @@ gtk_window_set_modal (GTK_WINDOW (priv->dialog), TRUE); gtk_dialog_add_button (GTK_DIALOG (priv->dialog), _("_Cancel"), + GTK_RESPONSE_CANCEL); idem
Review of attachment 269257 [details] [review]: ::: panels/privacy/privacy.ui @@ +126,3 @@ <property name="resizable">False</property> + <property name="use_header_bar">1</property> + <child internal-child="headerbar"> This isn't needed
Review of attachment 269264 [details] [review]: ::: panels/user-accounts/um-photo-dialog.c @@ +95,2 @@ _("_Cancel"), + GTK_RESPONSE_CANCEL, unrelated - please drop before pushing
Review of attachment 269265 [details] [review]: ::: panels/sound/gvc-mixer-dialog.c @@ +1591,2 @@ title = g_strdup_printf (_("Speaker Testing for %s"), gvc_mixer_ui_device_get_description (output)); d = gtk_dialog_new_with_buttons (title, If there are no buttons then gtk_dialog_new()
Review of attachment 269266 [details] [review]: we can't remove the close button on this one
Review of attachment 270783 [details] [review]: ::: panels/keyboard/gnome-keyboard-panel.ui @@ -43,3 @@ - <property name="visible">True</property> - <property name="can_focus">True</property> - <property name="can_default">True</property> Don't drop this, it causes a runtime warning.
Review of attachment 270784 [details] [review]: ::: panels/common/language-chooser.ui @@ +8,3 @@ <property name="resizable">False</property> + <property name="use_header_bar">1</property> + <child internal-child="headerbar"> None of this is needed
Review of attachment 270785 [details] [review]: ::: panels/region/format-chooser.ui @@ +10,3 @@ <property name="modal">True</property> + <property name="use_header_bar">1</property> + <child internal-child="headerbar"> No ::: panels/region/input-chooser.ui @@ +13,3 @@ + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="title" translatable="yes">Add an Input Source</property> Don't repeat this please
Review of attachment 270786 [details] [review]: ::: panels/universal-access/uap.ui @@ +779,3 @@ <property name="margin_top">6</property> <property name="xalign">0</property> + <property name="max_width_chars">60</property> unrelated, please drop ::: panels/universal-access/zoom-options.c @@ -592,3 @@ - w = WID ("closeButton"); - g_signal_connect (G_OBJECT (w), "clicked", - G_CALLBACK (zoom_option_close_dialog_cb), Remove the function too as it's unused now
The ones I didn't mark look fine. This will need a UI/string freeze break exception to land.
Just a question - we can to see this in 3.12? If it for 3.12 I'll find a time tomorrow to update all the patches, but if it will land in 3.13/3.14 I'll do it later.
Review of attachment 269252 [details] [review]: ::: panels/display/cc-display-panel.c @@ +1509,3 @@ gtk_window_set_modal (GTK_WINDOW (priv->dialog), TRUE); gtk_dialog_add_button (GTK_DIALOG (priv->dialog), _("_Cancel"), + GTK_RESPONSE_CANCEL); This isn't right. I needs to do this (to change to GTK_RESPONSE_CANCEL) if I want to hide the close button, and to put the buttons in the right place on the header bar. Without this change, I have close button on the end, and also two buttons at the end, "Cancel" and "Apply". If I change to GTK_RESPONSE_CANCEL, I hvae only "Cancel" button at the end, without close button, and "Apply" at the start of the header bar. @@ +1952,3 @@ gtk_window_set_modal (GTK_WINDOW (priv->dialog), TRUE); gtk_dialog_add_button (GTK_DIALOG (priv->dialog), _("_Cancel"), + GTK_RESPONSE_CANCEL); idem.
Created attachment 272732 [details] [review] notifications: Use header bar in the edit dialog
Created attachment 272733 [details] [review] display: Use header bar in the dialogs
Created attachment 272734 [details] [review] privacy: Use header bar in all the dialogs
Created attachment 272735 [details] [review] user-accounts: Use header bar in the photo dialog
Created attachment 272736 [details] [review] sound: Use header bar in the testing dialog
Created attachment 272737 [details] [review] user-accounts: Use header bar in the history dialog
Created attachment 272738 [details] [review] common: Use header bar in the language chooser dialog
Created attachment 272739 [details] [review] region: Use header bar in all the dialogs
Created attachment 272740 [details] [review] universal-access: Use header bar in all the dialogs
Created attachment 272741 [details] [review] keyboard: Use header bar in the custom shortcut dialog
Created attachment 272958 [details] [review] user-accounts: Use header bar in the account dialog
Created attachment 272959 [details] [review] user-accounts: Use header bar in the password dialog
Created attachment 272960 [details] [review] user-accounts: Use header bar in the photo dialog Same to comment #52, we needs to change to GTK_RESPONSE_CANCEL.
(In reply to comment #55) > Created an attachment (id=272960) [details] [review] > user-accounts: Use header bar in the photo dialog > > Same to comment #52, we needs to change to GTK_RESPONSE_CANCEL. Sorry, comment #42, not 52.
Ping (before 3.13.1).
Created attachment 275363 [details] [review] user-accounts: Remove unused callback
Review of attachment 275363 [details] [review]: Looks fine
Attachment 275363 [details] pushed as e168e2a - user-accounts: Remove unused callback