Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notification workflow #253

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Notification workflow #253

wants to merge 10 commits into from

Conversation

danirabbit
Copy link
Member

@danirabbit danirabbit commented Dec 1, 2022

Fixes #107
Closes #243
Closes #202
Closes #191

Opening this up to discussion! This is fairly large change to the screenshot workflow that takes a lot of responsibility off of Screenshot and puts more of it on Photos.

Instead of asking if and where we should save, we go ahead and save and send a notification. From the notification you can see where screenshots are saved by default.

Since we've saved a file, I'm not sure if we should just go ahead and copy that to your clipboard? Or offer a "Copy" action in the notification? and if that would help resolve issues like #224 and #156 to copy a file instead of image data?

If you choose to open the screenshot from the notification, you can then crop, edit, scale, save in a different location, save in a different file format, etc from Photos. This also makes it more obvious that markup features like #191 belong in Photos, not in the screenshot save dialog which no longer exists

Testers don't forget you can grab a Flatpak bundle if you don't want to manually build :) https://github.com/elementary/screenshot/actions/runs/3588339653

@danirabbit danirabbit requested a review from a team December 1, 2022 01:03
@TomiOhl
Copy link
Contributor

TomiOhl commented Dec 1, 2022

Interesting idea. Might just be me, but I liked that there was an option to only copy it, but don't save it. I think it's a fairly common use case to quickly screenshot something & send it & then never look at it again, saving it in these cases would just unnecessarily clutter up the storage. However, if the "screenshot and copy only" keyboard combination remains, this is not a deal braker to me.

I installed the package and tried it out, the screenshots do get saved but I don't receive the notification. Note that I have not uninstalled the default version, idk if that could be the culprit

@mpanhans
Copy link
Sponsor

mpanhans commented Dec 2, 2022

I prefer the current workflow in Screenshot for a number of reasons:

  1. The vast majority of screenshots I take are for copying and pasting into a messaging application or email. The "Copy to Clipboard" button is so quick for this. After taking the screenshot, you are only one click away from being able to paste it. There is no mental friction to consider where files are being saved or to remember to come back later and delete the file.
  2. In the rare cases when I do need to save a screenshot as a file, I almost never want it in some default folder. Screenshot lets me quickly name the image and choose its location. A default folder for screenshots runs the risk of becoming a forgotten storage locker of screenshot items of varying importance. Like the "Downloads" folder becomes for many people.
  3. I'm not quite seeing the rationale for using a notification workflow for Screenshot. Typically notifications are used to update someone about the status of an app. When I take a screenshot, I am not really waiting to be updated about the resulting action or a change in some status. I know what to expect and when to expect it. To me, the current workflow without notifications seems appropriate.

In terms of the test build, similar to above I also do not receive any notification upon taking a screenshot. The screenshots do get saved in Pictures > Screenshots. Though I'd say there should probably be a way to change the default location for saving.

@jeremypw
Copy link
Collaborator

jeremypw commented Feb 1, 2023

I think it is a good idea to be able to open the screenshot in Photos easily and I like the idea that screenshots can (in theory) be accessed from notifications after screenshot dialog has closed. However, this implementation has a number of flaws in my view:

  1. The notification disappears quite quickly so it is easy to miss the opportunity to open the screenshot in Photos
  2. The notification in the list gives the folder where to find the file but clicking on it just reopens screenshot dialog which does not allow you to do anything useful.
  3. The screenshot is neither copied into the clipboard nor is there any UI to allow you to do so.
  4. Notifications were not received when run as a Flatpak

A common workflow for me is to screenshot an app and then include it in a GitHub comment so if a way can be found to do that that does not involve manually finding the file in the Screenshots folder that would be good. Maybe have a list of screenshot objects that can just be dragged into the Github comment or selected and pasted. The same objects could have a button to open them in Photos for editing

@@ -270,57 +265,36 @@ public class Screenshot.ScreenshotWindow : Hdy.ApplicationWindow {
} while (File.new_for_path (full_file_name).query_exists ());

screenshot.save (full_file_name, format);

var file_icon = new FileIcon (File.new_for_path (full_file_name));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not getting a notification when running as Flatpak although it works OK native. It seems to be linked to using a FileIcon (or at least, this one). Using a ThemedIcon works OK in Flatpak.

@mpanhans
Copy link
Sponsor

To easily open Photos, why not just add one button to the screenshot window with that option? Currently, there are three buttons at the bottom of the window after a screenshot has been taken: "Copy to Clipboard", "Cancel", and "Save". One more button could be added that says "Open in Photo Application" and it can open the image in the system's default image viewer.

A common workflow for me is to screenshot an app and then include it in a GitHub comment so if a way can be found to do that that does not involve manually finding the file in the Screenshots folder that would be good.

The easiest and fastest way to do this seems to be taking a screenshot, click "Copy to Clickboard", and then hit paste in the Github comment. Adding notifications or saving the file by default seems to only add frictions, no?

@zeebok
Copy link
Contributor

zeebok commented May 13, 2023

Maybe this is a better workflow for the screenshot keyboard shortcut rather than the when using the app itself? If I could just hit the key and have the option to copy it to clipboard instead of it saving would be great. Opening the app and using it feels like there much more intention from a user and so should make less assumptions and let them do what they intend.

@jeremypw
Copy link
Collaborator

@danirabbit Are you intending to press ahead with this change, perhaps modified in the light of comments, at some point?

@danirabbit danirabbit marked this pull request as draft July 21, 2023 12:39
@danirabbit
Copy link
Member Author

@jeremypw yeah converted to draft and added to the OS 8 board. I’d like to revisit this but it’s low priority for me right now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants