Created attachment 273291 [details] [review]
[PATCH] Properties dialog should have a X close button instead of  "Done"

My smallest patch ever.

Review of attachment 273291 [details] [review]:

Thanks for the patch Felipe! Could you please add the URL to this bug in the commit message? You can consider git-bz for submitting patches, if you want to.

::: src/properties.js
@@ -64,1 @@
         this.widget.set_default_response(Gtk.ResponseType.CLOSE);

You need to remove the set_default_response call too. The dialog no longer has a widget that will emit CLOSE. Pressing ESC or the close button emits DELETE_EVENT.

(In reply to comment #2)
> ::: src/properties.js
> @@ -64,1 @@
>          this.widget.set_default_response(Gtk.ResponseType.CLOSE);
> 
> You need to remove the set_default_response call too. The dialog no longer has
> a widget that will emit CLOSE. Pressing ESC or the close button emits
> DELETE_EVENT.

Or maybe change the set_default_response to DELETE_EVENT so that pressing ENTER after changing the title continues to close the dialog? It will still close the dialog with your current patch, but I don't see the need to emit two different responses because the dialog is "instant apply".

Created attachment 273333 [details] [review]
Properties dialog should have a X close button instead of "Done"

Review of attachment 273333 [details] [review]:

::: src/properties.js
@@ -62,3 @@
                                         hexpand: true });
-        this.widget.add_button(_("Done"), Gtk.ResponseType.CLOSE);
-        this.widget.set_default_response(Gtk.ResponseType.CLOSE);

Sorry for misleading you Felipe. Setting the default_response to DELETE_EVENT won't work because there is no button associated with it anymore. The user either has to hit ESC or click X. Hitting ENTER in the entry won't work.

Created attachment 273334 [details] [review]
properties: Use a X close button instead of "Done"

Fixed up your last patch to not set the default_response to DELETE_EVENT and committed it.

Thanks for working on this.