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 765941 - Add backbutton after screenshot
Add backbutton after screenshot
Status: RESOLVED FIXED
Product: gnome-screenshot
Classification: Core
Component: general
3.18.x
Other Linux
: Normal enhancement
: ---
Assigned To: gnome-screenshot-maint
gnome-screenshot-maint
: 730618 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-05-03 13:52 UTC by martinojones_2009
Modified: 2017-09-21 19:54 UTC
See Also:
GNOME target: ---
GNOME version: 3.17/3.18


Attachments
Updated to support back button after screenshot (4.62 KB, patch)
2017-04-25 03:10 UTC, Daniel Byrne
reviewed Details | Review
added in changes from review (2.66 KB, patch)
2017-07-29 18:21 UTC, Daniel Byrne
reviewed Details | Review
complete patch for back button (4.06 KB, patch)
2017-07-31 02:50 UTC, Daniel Byrne
reviewed Details | Review

Description martinojones_2009 2016-05-03 13:52:31 UTC
Can a back button be added to GNOME-screenshot? After someone takes a screenshot of an area it would be cool if they could click a back button instead of closing the application and reopening it if someone took a bad screenshot they didn't want to save.
Comment 1 Peter Sonntag 2017-01-07 10:11:39 UTC
It would be really helpful, if there would be an option (E.g. additional button) in order to take an other screenshot.
Comment 2 Daniel Byrne 2017-04-25 03:10:09 UTC
Created attachment 350353 [details] [review]
Updated to support back button after screenshot
Comment 3 Cosimo Cecchi 2017-04-30 23:21:39 UTC
Review of attachment 350353 [details] [review]:

Thanks, this looks pretty good. Only a couple of minor comments below.

::: src/screenshot-application.c
@@ +402,3 @@
+  save_folder_to_settings (self);
+  gtk_widget_destroy (dialog->dialog);
+  g_free (dialog);

Could you factor out these lines into a separate function? It looks like they're also called from https://git.gnome.org/browse/gnome-screenshot/tree/src/screenshot-application.c#n114

@@ +432,3 @@
       break;
+    case SCREENSHOT_RESPONSE_BACK:
+      screenshot_back(self);

Add missing space before paren
Comment 4 Daniel Byrne 2017-07-29 18:21:32 UTC
Created attachment 356563 [details] [review]
added in changes from review

applied changes from code review
Comment 5 Cosimo Cecchi 2017-07-30 13:57:43 UTC
Daniel, thanks for the updated patch!
However, I notice that you have only attached a diff from the previous version to the one with the latest changes.

Instead, please re-attach the whole diff as a single git-formatted patch. (See https://wiki.gnome.org/Git/Developers#Contributing_patches for more information on how you can accomplish that.)
Comment 6 Daniel Byrne 2017-07-31 02:50:31 UTC
Created attachment 356621 [details] [review]
complete patch for back button
Comment 7 Cosimo Cecchi 2017-07-31 10:15:38 UTC
Thanks! I tweaked a bit the layout and used an icon button instead of a textual one, since that's what all the other GNOME apps use for this pattern, and committed the patch to master.
Comment 8 Cosimo Cecchi 2017-09-21 19:54:32 UTC
*** Bug 730618 has been marked as a duplicate of this bug. ***