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 747381 - Ask for the directory name before creating it
Ask for the directory name before creating it
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: Views: All
3.16.x
Other Linux
: Normal enhancement
: 3.18
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-04-05 18:31 UTC by Georges Basile Stavracas Neto
Modified: 2015-04-20 13:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
view: show "New Folder" dialog (10.08 KB, patch)
2015-04-06 19:47 UTC, Georges Basile Stavracas Neto
none Details | Review
view: show "New Folder" dialog (17.24 KB, patch)
2015-04-19 01:11 UTC, Georges Basile Stavracas Neto
none Details | Review
view: show "New Folder" dialog (17.26 KB, patch)
2015-04-19 12:58 UTC, Georges Basile Stavracas Neto
none Details | Review
view: show "New Folder" dialog (16.67 KB, patch)
2015-04-20 11:12 UTC, Georges Basile Stavracas Neto
none Details | Review
view: show "New Folder" dialog (16.71 KB, patch)
2015-04-20 12:13 UTC, Georges Basile Stavracas Neto
none Details | Review
view: show "New Folder" dialog (16.66 KB, patch)
2015-04-20 12:14 UTC, Georges Basile Stavracas Neto
none Details | Review
view: show "New Folder" dialog (17.09 KB, patch)
2015-04-20 12:55 UTC, Georges Basile Stavracas Neto
none Details | Review
view: show "New Folder" dialog (17.19 KB, patch)
2015-04-20 13:27 UTC, Georges Basile Stavracas Neto
none Details | Review
view: show "New Folder" dialog (17.14 KB, patch)
2015-04-20 13:32 UTC, Georges Basile Stavracas Neto
none Details | Review
view: show "New Folder" dialog (17.14 KB, patch)
2015-04-20 13:39 UTC, Georges Basile Stavracas Neto
committed Details | Review

Description Georges Basile Stavracas Neto 2015-04-05 18:31:07 UTC
The new mockups include a new "New Folder" dialog.
It should be implemented in order to achieve the (so needed) GNOME modernization.
Comment 1 Georges Basile Stavracas Neto 2015-04-06 19:47:18 UTC
Created attachment 301034 [details] [review]
view: show "New Folder" dialog

This commit introduces the "New Folder"
dialog, which asks the folder name before
actually creating it.

With the introduced changes, the folder
is created with the given name instead
of creating it first with the generic
"Unamed folder" and then renaming it.

This dialog is part of the ongoing effort
to modernize Nautilus to better fit GNOME
standards, and the latest mockups can
be found at [1].

[1] https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/nautilus/nautilus-next/new-folder.png
Comment 2 Carlos Soriano 2015-04-08 12:03:22 UTC
Review of attachment 301034 [details] [review]:

I think is better to use GtkBuilder files whenever possible, and this is a clear one even if it is a simple dialog... =)
Like action_show_move_to_trash_shortcut_changed_dialog does more or less.

On the other hand, here we can be smarter, since we can now show a hint that the name is invalid and disable the OK button.
The valid names are controlled in nautilus_file_rename in nautilus-file.c . You could extract that part of the code into a function like "validate_file_name" or so.
Then call it from the validate_entry, and if not correct make the hint with a red entry and a dimmed text below the entry or something like that. That will look awesome! =) You can ask designers for this, and if you don't feel like doing it, you can tell me and skip it. But is a good moment for implement this.

::: libnautilus-private/nautilus-file-undo-operations.c
@@ +730,3 @@
 	parent = g_file_get_parent (self->priv->target_file);
 	parent_uri = g_file_get_uri (parent);
+	nautilus_file_operations_new_folder (NULL, NULL, parent_uri, NULL,

NULL? then the folder name is "Untitled folder"... You will need a way here to use the old name.

::: src/nautilus-view.c
@@ +1599,3 @@
+	const gchar *text;
+
+	g_assert (object && GTK_IS_ENTRY (object));

we usually use just g_assert(GTK_IS_ENTRY (object)), but yours looks better to me.
Good idea

@@ +1606,3 @@
+	gtk_dialog_set_response_sensitive (GTK_DIALOG (user_data),
+                                           GTK_RESPONSE_OK,
+	g_assert (object && GTK_IS_ENTRY (object));

I guess g_utf8_strlen (text, 1) should be enough no?

@@ +1620,3 @@
+	text = gtk_entry_get_text (GTK_ENTRY (entry));
+
+                                           g_utf8_strlen (text, -1) > 0);

If the button is inactive in validate_entry, this check shouldn't be necessary.

If you want to keep it, I would put here a g_warning in an else statement, since seeing the button be able to be activated but doing nothing doesn't looks good in any case.

@@ +1640,1 @@
+	// Dialog label

comments use the c89 style, /* */

@@ +1698,3 @@
+
+		data = new_folder_data_new (directory_view, with_selection);
+	gtk_container_add (GTK_CONTAINER (box), entry);

From docs: "a pointer to the contents of the widget as a string. This string points to internally allocated storage in the widget and must not be freed, modified or stored."

Although we don't destroy it until we set the name in nautilus_file_operations_new_folder, I think is better to  g_strdup always in these cases.

@@ +1714,3 @@
+                                                     parent_uri,
+                                                     name,
+                          G_CALLBACK (nautilus_view_add_file_dialog_validate_entry),

You don't select the file and scroll to it, as it was doing before.
Comment 3 Carlos Soriano 2015-04-08 12:49:20 UTC
This should fix it

Attachment 301034 [details] pushed as 6f4fc64 - view: show "New Folder" dialog
Comment 4 Carlos Soriano 2015-04-08 12:50:57 UTC
whops, obviously my intention was not to push it
Comment 5 Carlos Soriano 2015-04-09 08:01:51 UTC
Review of attachment 301034 [details] [review]:

chaging status of patch
Comment 6 Georges Basile Stavracas Neto 2015-04-19 01:11:21 UTC
Created attachment 301922 [details] [review]
view: show "New Folder" dialog

With the introduced changes, the folder
is created with the given name instead
of creating it first with the generic
"Unamed folder" and then renaming it.

The dialog has a "delayed message" logic,
where typos are immediatly recognized, but
coincident folder names appear ~2s later.
Comment 7 Georges Basile Stavracas Neto 2015-04-19 12:58:01 UTC
Created attachment 301928 [details] [review]
view: show "New Folder" dialog

Fixed a memory leak and fixed a typo.
Comment 8 Carlos Soriano 2015-04-20 09:45:34 UTC
Review of attachment 301928 [details] [review]:

Looks almost finished. Just some nitpicks.

::: src/nautilus-new-folder-dialog.ui
@@ +13,3 @@
+      <object class="GtkBox" id="vbox">
+        <property name="orientation">vertical</property>
+        <property name="border_width">24</property>

little less padding in top and bottom, as per the mockups. maybe 10? I normally measure them with gimp from the mockups.

@@ +21,3 @@
+            <property name="xalign">0</property>
+            <style>
+              <class name="dim-label"/>

its not dimmed in the mockups

@@ +44,3 @@
+        </child>
+        <child>
+          <object class="GtkRevealer" id="revealer">

looks nice, but actually we don't want to resize the window, as per the mockups. Just make the label empty.

@@ +87,3 @@
+    </child>
+    <action-widgets>
+      <action-widget response="apply" default="true">create_button</action-widget>

response="ok"

::: src/nautilus-view.c
@@ +1634,3 @@
+
+static gboolean
+has_folder_cb (NewFolderDialogData *data)

not needed to use _cb (in this class is not used much, and your other functions don't use it like activate, etc.)

@@ +1647,3 @@
+
+static void
+nautilus_view_add_file_dialog_validate_content (GObject    *object,

validate_name looks more specific here.

@@ +1661,3 @@
+	gboolean has_folder;
+
+	g_assert (object && GTK_IS_ENTRY (object));

*_IS_* (NULL) works fine. Not need to check for null.

@@ +1662,3 @@
+
+	g_assert (object && GTK_IS_ENTRY (object));
+	g_assert (user_data && data->view && data->dialog && data->revealer);

I would say one per line, and you can use *_IS_*.

@@ +1694,3 @@
+			gtk_label_set_label (GTK_LABEL (label), _("A folder with that name already exists."));
+		} else {
+			view->details->new_folder_name_timeout_id = g_timeout_add (1750,

use a constant. And I would say 1 is ok... almost two seconds feel too long (although I know the design says 2 or 3)

@@ +1710,3 @@
+
+	gtk_dialog_set_response_sensitive (GTK_DIALOG (dialog),
+                                           GTK_RESPONSE_APPLY,

nitpicking, but I think here looks better GTK_RESPONSE_OK semantically (you are not aplying a prefenrece or something and letting the window open, but instead you are done and the window is closed.)

@@ +1733,3 @@
+	 * it all again. Checking if the "Create" button is sensitive
+	 * is enought and sufficiently fast to determine if the dialog
+	 * can create this folder.

redo/check
enought/enough

And I would remove the last part after "is enough"

@@ +1739,3 @@
+                                     GTK_RESPONSE_APPLY);
+	} else {
+		g_warning ("Invalid folder name.");

I would say, set the error label immediately if this happens instead of a warning where the user can't see it.

@@ +1772,3 @@
+                                          "activated_cb",
+                                          G_CALLBACK (nautilus_view_add_file_dialog_entry_activate),
+                                          NULL);

Just curious, is this necesary and not enough with gtk_builder_connect_signals and the application symbol table?

@@ +1776,3 @@
+	gtk_builder_connect_signals (builder, dialog_data);
+
+	/* run the dialog */

comment not needed

@@ +1787,3 @@
+		/* To avoid any inconsistencies with the
+		 * typed folder name, duplicate it.
+		 */

I don't think the comment is necessary. Basically if you don't own something and you need to operate with it with the risk of it being freed, duplicate it
Comment 9 Georges Basile Stavracas Neto 2015-04-20 11:12:11 UTC
Created attachment 301983 [details] [review]
view: show "New Folder" dialog

Proposed changes applied.

One thing that is bothering me is that this patch
does one I/O block per check, and a check each time
the folder name changes. I couldn't figure out how
to fix it without adding a major maintance burden.
Comment 10 Georges Basile Stavracas Neto 2015-04-20 12:13:20 UTC
Created attachment 301987 [details] [review]
view: show "New Folder" dialog

As Carlos suggested, use the NautilusDirectory to
check for file existance. This saves us some (maybe
several) disk accesses.
Comment 11 Georges Basile Stavracas Neto 2015-04-20 12:14:49 UTC
Created attachment 301988 [details] [review]
view: show "New Folder" dialog

Remove an spurious warning
Comment 12 Georges Basile Stavracas Neto 2015-04-20 12:55:40 UTC
Created attachment 301990 [details] [review]
view: show "New Folder" dialog

Shows the label when the user tries to add
a folder with collidant name.
Comment 13 Carlos Soriano 2015-04-20 13:05:42 UTC
Review of attachment 301990 [details] [review]:

Make the dialog a little wider as per the mockups (and also it doesn't get resized when the warning is displayed) and looks good to commit.
Comment 14 Georges Basile Stavracas Neto 2015-04-20 13:27:27 UTC
Created attachment 301993 [details] [review]
view: show "New Folder" dialog

Make the dialog at least 50 chars wide.
Comment 15 Georges Basile Stavracas Neto 2015-04-20 13:32:57 UTC
Created attachment 301994 [details] [review]
view: show "New Folder" dialog

As Carlos requested, use GtkWidget::width-request property.
Comment 16 Georges Basile Stavracas Neto 2015-04-20 13:39:55 UTC
Created attachment 301995 [details] [review]
view: show "New Folder" dialog

Reduce timeout for error label. 1s is still too slow.
Comment 17 Carlos Soriano 2015-04-20 13:42:17 UTC
Review of attachment 301995 [details] [review]:

LGTM thanks!
Comment 18 Georges Basile Stavracas Neto 2015-04-20 13:45:04 UTC
Attachment 301995 [details] pushed as 6ae3384 - view: show "New Folder" dialog