Created attachment 206028 [details] [review]
screenshot: add a 'flash' boolean flag to screenshot methods

Add a flag to these methods that allows flashing the area of the
screenshot directly from the compositor.

Review of attachment 206028 [details] [review]:

Looks good and seems to work fine here.

::: js/ui/flashspot.js
@@ +24,3 @@
+        Tweener.addTween(this.actor,
+                         { opacity: 255,
+                           time: 0.3,

Use a constant rather then a hardcoded value.

@@ +31,3 @@
+    },
+
+    _fireShowComplete: function() {

Should be _onFireShowComplete

::: js/ui/shellDBus.js
@@ +137,3 @@
+        let [x, y, width, height, flash, filename, callback] = params;
+        global.screenshot_area (x, y, width, height, flash, filename, Lang.bind(this,
+            function (obj, result, area) {

We should move this into a named function and not avoid having it duplicated multiple times (should probably be a separate patch though).

::: src/shell-global.c
@@ +2082,3 @@
   cairo_surface_mark_dirty (screenshot_data->image);
 
+  screenshot_data->screenshot_area.x = screenshot_data->x;

Just kill x, y, width and height and always use the area. This feels kind of redundant now.

(In reply to comment #2)

> We should move this into a named function and not avoid having it duplicated

s/not// ;)

Created attachment 206100 [details] [review]
screenshot: add a 'flash' boolean flag to screenshot methods

Addresses review comments and cleans up the unused "flash" gboolean parameter in the C code.

Created attachment 206101 [details] [review]
shell-dbus: factor screenshot callback into a separate function

Share the screenshot methods callback into a factored out function.

(In reply to comment #2)
> Review of attachment 206028 [details] [review]:
> 
> Looks good and seems to work fine here.

Thanks for the review; I addressed your points in the two patches I just attached.

Review of attachment 206100 [details] [review]:

Looks good.

Review of attachment 206101 [details] [review]:

This too.

Attachment 206100 [details] pushed as 049a561 - screenshot: add a 'flash' boolean flag to screenshot methods
Attachment 206101 [details] pushed as ef56a78 - shell-dbus: factor screenshot callback into a separate function

Thanks, pushed.