GNOME Bugzilla – Bug 723793
Use header bars for dialogs
Last modified: 2014-02-07 14:38:18 UTC
Here is a patch to use header bars in the dialogs used by Nautilus.
Created attachment 268336 [details] [review] Use header bars for dialogs
Review of attachment 268336 [details] [review]: Thanks! Looks mostly good - I left some comments below. ::: libnautilus-private/nautilus-file-conflict-dialog.c @@ +626,3 @@ dialog = GTK_WIDGET (g_object_new (NAUTILUS_TYPE_FILE_CONFLICT_DIALOG, + "use-header-bar", TRUE, + "modal", TRUE, Should this really be modal? You might want to use the window behind to get more information about the files conflicting ::: src/nautilus-connect-server-dialog.c @@ -45,3 @@ GtkWidget *remove_menu_item; GtkWidget *clear_menu_item; - GtkWidget *browse_button; Why did you remove this? ::: src/nautilus-file-management-properties.ui @@ -1300,3 @@ - <action-widget response="-11">helpbutton1</action-widget> - <action-widget response="-7">closebutton1</action-widget> - </action-widgets> You should also remove the code that connects to the "response" signal inside nautilus-file-management-properties.c then. ::: src/nautilus-properties-window.c @@ +4849,3 @@ + window = NAUTILUS_PROPERTIES_WINDOW (gtk_widget_new (NAUTILUS_TYPE_PROPERTIES_WINDOW, + "modal", TRUE, Not sure I agree this should be modal... you sometimes want to e.g. compare properties for two similar files or folders in the same directory. @@ +4925,3 @@ } + gtk_container_set_border_width (GTK_CONTAINER (window), 0); Hmm, the border width should be 0 by default, so I think this is not needed (same below)
Review of attachment 268336 [details] [review]: ::: libnautilus-private/nautilus-file-conflict-dialog.c @@ +626,3 @@ dialog = GTK_WIDGET (g_object_new (NAUTILUS_TYPE_FILE_CONFLICT_DIALOG, + "use-header-bar", TRUE, + "modal", TRUE, OK, I've removed it for now. ::: src/nautilus-connect-server-dialog.c @@ -45,3 @@ GtkWidget *remove_menu_item; GtkWidget *clear_menu_item; - GtkWidget *browse_button; The connect to server option is now placed beside the browse network item in the sidebar. From the command line the browse was disabled anyway. The browse feature isn't that useful at the moment for a few reasons. And it complicates the actions available in the header bar - and it isn't completely clear what browse means there (eg. if it acts on the current data entered or not). Better to just make the dialog do one thing I think. ::: src/nautilus-file-management-properties.ui @@ -1300,3 @@ - <action-widget response="-11">helpbutton1</action-widget> - <action-widget response="-7">closebutton1</action-widget> - </action-widgets> Done. ::: src/nautilus-properties-window.c @@ +4925,3 @@ } + gtk_container_set_border_width (GTK_CONTAINER (window), 0); Right, the only one needed is the content area one it seems.
Created attachment 268413 [details] [review] Use header bars for dialogs
Review of attachment 268413 [details] [review]: ::: libnautilus-private/nautilus-file-conflict-dialog.c @@ +626,3 @@ dialog = GTK_WIDGET (g_object_new (NAUTILUS_TYPE_FILE_CONFLICT_DIALOG, + "use-header-bar", TRUE, + "modal", TRUE, Didn't you agree to drop this one? ::: src/nautilus-file-management-properties.c @@ -737,3 @@ dialog = GTK_WIDGET (gtk_builder_get_object (builder, "file_management_dialog")); - g_signal_connect_data (dialog, "response", - G_CALLBACK (nautilus_file_management_properties_dialog_response_cb), The callback body should be removed too, this is the only place where it was referenced AFAICS
Created attachment 268415 [details] [review] Use header bars for dialogs
Sorry, too many patches flying around here... I removed the modal from the properties dialog. I got confused with the commenting in splinter. I don't want to remove modal from the conflict dialog. It presents context directly in the dialog and if it isn't enough we should add to it. The action is inherently modal. It stops so you don't mess up your files. And if there is any concern it should be canceled or skipped.
Review of attachment 268415 [details] [review]: Cool; your rationale about the file conflict dialog makes sense, and this looks good now.
Attachment 268415 [details] pushed as d8a8ab3 - Use header bars for dialogs