GNOME Bugzilla – Bug 682637
Improve the connect to server dialog
Last modified: 2012-09-02 23:36:32 UTC
The current connect to server dialog has a number of problems. * It is sometimes invoked as a separate process but it is not an app * It prompts for information in separate fields that vary by connection type * It is not clear which fields are required * Some required fields are actually not required. For example the dialog doesn't know if anonymous or guest logins are supported until it tries. * The dialog duplicates a lot of UI that we have worked to polish already (mount operations dialog etc) * The code behind the dialog is separate from the code that normally does mounting when using the location bar or using the Network place. * There is no way to browse for connections from the dialog * There is no recently used list to help you quickly reconnect to something that you used before but didn't bookmark * It is unclear whether the server field can be a URI or must only be a hostname * Showing port values by default is weird and confusing * Particularly when it is decoupled from and presented differently from the connection type dropdown * The connection type dropdown is highly problematic. * It changes values above it (the port number) which is a weird thing to do * It causes the rest of the dialog to change unpredictably * It doesn't contain a complete set of protocols that are supported * It isn't autodetected if I type in a URI into the server box And perhaps subtle but important: * The dialog is very different from the connect to server dialogs in both Windows and OS X. Online help for how to connect to a service will show one or both of those. Being consistent with them makes a lot of sense. See http://www.synology.com/tutorials/how_to_intranet.php?lang=us#t3_2 for an example.
Created attachment 222364 [details] [review] New design for the connect to server dialog
*** Bug 139105 has been marked as a duplicate of this bug. ***
Review of attachment 222364 [details] [review]: Looks pretty good...some comments below; if we want to get this in for 3.6, we should ask for an UI freeze break. ::: src/nautilus-application.c @@ +846,3 @@ + if (error != NULL) { + g_warning ("Unable to open server bookmarks: %s", error->message); + char *uri; Wouldn't this spawn a warning the first time the dialog is used? I think we should only warn out when the file exists already (don't know if you can detect this from the GError code or if you need to check it with g_file_test() or similar). ::: src/nautilus-connect-server-dialog.c @@ +468,3 @@ + if (uri != NULL) { + server_list_remove (dialog, uri); + gtk_list_store_remove (dialog->details->store, &iter); You're leaking the uri here
Created attachment 222408 [details] [review] New design for the connect to server dialog
Created attachment 222409 [details] [review] New design for the connect to server dialog Also allow activation from the recent list.
Review of attachment 222409 [details] [review]: Code wise, I think this looks basically ready to go in. There are some issues we have to figure out though: - we need to ask a freeze break - there is a number of applications (a grep in my checkouts directory returned at least baobab, gnome-shell and gnome-panel) that currently rely on nautilus-connect-server being available, and this patch removes the binary. Even if we are granted a freeze break, we might be a little tight on time to fix this in all the other applications; if you want to try merging this for this release, I propose we keep the nautilus-connect-server separate binary (maybe removing its desktop file) for now, and think of a better solution for other applications during next cycle. ::: src/nautilus-connect-server-dialog.c @@ +140,3 @@ + g_free (scheme); + } else { + g_set_error_literal (error, Wrong indentation for this block @@ +249,2 @@ + uri = g_strdup_printf ("%s://foo.example.com", get_default_scheme (dialog)); + text = g_strdup_printf (_("For example, %s"), uri); I think this needs a comment for translators.
Created attachment 222459 [details] [review] New design for the connect to server dialog
(In reply to comment #6) > - there is a number of applications (a grep in my checkouts directory returned > at least baobab, gnome-shell and gnome-panel) that currently rely on > nautilus-connect-server being available, and this patch removes the binary. > Even if we are granted a freeze break, we might be a little tight on time to > fix this in all the other applications; if you want to try merging this for > this release, I propose we keep the nautilus-connect-server separate binary > (maybe removing its desktop file) for now, and think of a better solution for > other applications during next cycle. I would like to try to get this in. One problem is that nautilus-connect-server doesn't actually have a desktop file so the shell and panel hacks it in. I think it should simply be removed from the shell. We really don't want that Connect To.. item showing up in the shell regardless of this work. Baobab is a little tricker since it expects the binary to mount the remote directory for it as well. The new connect dialog doesn't do this. We'd have to add all that duplicated mount code back into the standalone binary thing. Honestly, I'm not sure that using baobab on a remote system is all that great of an idea. But why don't we just say that you'd have to connect up a mount in nautilus before you can select it in baobab? And then you can just pick it as a regular folder. So, I'd propose removing the connect to remote thing for now. For the panel we don't actually have to modify any code. It uses: if (panel_is_program_in_path ("nautilus-connect-server")) { So that's fine for now.
Created attachment 222581 [details] [review] New design for the connect to server dialog
Created attachment 222582 [details] [review] Add back a legacy nautilus-connect-server command In order to ease the transition.
Created attachment 222583 [details] [review] Don't show the browse button for the connect to server standalone
Review of attachment 222581 [details] [review]: Looks good, but we'll need r-t approval for this
Review of attachment 222583 [details] [review]: ++
Review of attachment 222582 [details] [review]: ++
Would be great to have before/after screenshots.
(In reply to comment #15) > Would be great to have before/after screenshots. Yeah, we're working on some smaller refinements of the UI before posting screenshots the formal freeze break request.
(In reply to comment #16) > Yeah, we're working on some smaller refinements of the UI before posting > screenshots the formal freeze break request. "...and the formal freeze break request."
Created attachment 222662 [details] [review] New design for the connect to server dialog
Created attachment 222663 [details] [review] Add back a legacy nautilus-connect-server command In order to ease the transition.
Created attachment 222664 [details] [review] Don't show the browse button for the connect to server standalone
Created attachment 222666 [details] screenshot with patches
So I guess the idea here is that let the user enter a single url like foo://server.name:port/path/to/nowhere instead of breaking that up into server, protocol, port and folder controls. Makes sense to me, +1
Review of attachment 222662 [details] [review]: Okay
Review of attachment 222663 [details] [review]: ++
Review of attachment 222664 [details] [review]: ++
Created attachment 222679 [details] old dialog for reference
Does the new dialog work with \\foo\bar addresses for samba shares? Most often the windows/samba shares are advertised this way rather than using an smb:// uri. Are the credentials asked afterwards?
(In reply to comment #27) > Does the new dialog work with \\foo\bar addresses for samba shares? Most often > the windows/samba shares are advertised this way rather than using an smb:// > uri. These changes gives us an opportunity to address that where we could not before. See bug 446136 > Are the credentials asked afterwards? Yes.
Attachment 222662 [details] pushed as 95d42ea - New design for the connect to server dialog Attachment 222663 [details] pushed as cbca1bf - Add back a legacy nautilus-connect-server command Attachment 222664 [details] pushed as 9cecb29 - Don't show the browse button for the connect to server standalone Pushed with a minor modification to keep the binary in $PATH after r-t approval.