Created attachment 234951 [details] [review]
Add "Copy to the Clipboard" functionality

Review of attachment 234951 [details] [review]:

Thanks for the patch. Code here looks good, but I'm not 100% sure about the placement of the Copy action though. It would probably be better in a contextual menu, even though a menu with only one action might look a bit odd right now. Jon, what do you think?

Hey Fabiano, I talked with Jon about this today and we settled on a contextual menu for this.
The menu right now would only have the Copy action - in the future we might also want to expose an action that bookmarks/annotates the current selection.

Fabiano, any chance you can make an updated patch for this?

Created attachment 240064 [details] [review]
Add "Copy to the Clipboard" functionality

Review of attachment 240064 [details] [review]:

Thanks for the patch; I have a comment on the approach below:

::: src/preview.js
@@ +337,3 @@
 
+    _createPopupMenu: function() {
+        this._popupMenu = new Gtk.Menu();

Instead of creating the GtkMenuItem manually and connecting to activate, you could make a preview-context-menu.ui file in src/resources, load it here with gtk_menu_new_from_model().
That would automatically work with the action activation callback you install above.

(In reply to comment #6)
> Review of attachment 240064 [details] [review]:
> 
> Thanks for the patch; I have a comment on the approach below:
> 
> ::: src/preview.js
> @@ +337,3 @@
> 
> +    _createPopupMenu: function() {
> +        this._popupMenu = new Gtk.Menu();
> 
> Instead of creating the GtkMenuItem manually and connecting to activate, you
> could make a preview-context-menu.ui file in src/resources, load it here with
> gtk_menu_new_from_model().
> That would automatically work with the action activation callback you install
> above.

Cosimo,

For some reason that I didn't realize, the "Copy" menu item is not enabled.
Could you take a look and point me what I am missing?

Created attachment 240082 [details] [review]
Add "Copy to the Clipboard" functionality

Created attachment 240087 [details] [review]
Add "Copy to the Clipboard" functionality

Attachment 240087 [details] pushed as f9364e4 - Add "Copy to the Clipboard" functionality

The patch was only missing a call to gtk_menu_attach_to_widget(), which is necessary for gtk_menu_new_from_model() to work...
Quoting from the docs:
"The created menu items are connected to actions found in the GtkApplicationWindow to which the menu belongs - typically by means of being attached to a widget (see gtk_menu_attach_to_widget()) that is contained within the GtkApplicationWindows widget hierarchy."

I made that change and pushed the patch to master, thanks Fabiano!

*** Bug 698583 has been marked as a duplicate of this bug. ***