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

feat: add screenshot plugin #451

Merged
merged 154 commits into from
Nov 28, 2024
Merged

Conversation

amochuko
Copy link
Contributor

@amochuko amochuko commented Nov 19, 2024

PR to merge screenshot plugin

@amochuko
Copy link
Contributor Author

4.) Cancel an existing flow

I have an unfinished upload from before - whenever I click Swarm Screenshot, it takes me back to where I was in the previous flow. My two options are "Crop" and "Publish to Swarm". I would like to Cancel instead and start anew.

Hello. please I need more clarification of the above:

Because this was my thought process: whenever I have a work flow in progress, and I click Swarm Screenshot, I want my attention to where the flow is. And this also brings the window to focus incase it was behind other applications

Which of these step is best approach:

  1. Do I add a Cancel button to the window to which when clicked will reset the work flow?

@Cafe137
Copy link
Collaborator

Cafe137 commented Nov 25, 2024

Hello. please I need more clarification of the above:

It seems that I have written this when I reached an errored out state due to 3.)
Whenever I closed the window and clicked Swarm Screenshot, it got me back to the previous state with no Cancel button.
I think it is working now as intended as long as there are no prior errors.

@amochuko
Copy link
Contributor Author

amochuko commented Nov 25, 2024

Hello! Thanks for the PR, it is very nicely put together!

I have some questions and some minor issues I encountered:

1.) License

  • src/plugins/screenshot/audio/camera_shutter.m4a
  • src/plugins/screenshot/windows/preview/copy.svg

What are the licensings on these two files? We need to be extra careful to only accept public domain, CC0 assets.

2.) Negative file size

In the preview screen, I see negative values for file sizes, e.g.:

Resolution: 1172 x 886 File Size: -35.86 KB

3.) handleFileUpload errors out with encrypt

[desktop] time="2024-11-25T07:45:55.808Z" level="error" msg="handleFileUpload error: "
[desktop] Error occurred in handler for 'upload-to-swarm': TypeError: Only 32-byte, non-encrypted references are supported

This is caused by trying to convert the Swarm reference to a CID. That whole piece can be removed, we do not really use CIDs. It is OK to only use the Swarm references (a.k.a. Swarm hashes)

4.) Cancel an existing flow

I have an unfinished upload from before - whenever I click Swarm Screenshot, it takes me back to where I was in the previous flow. My two options are "Crop" and "Publish to Swarm". I would like to Cancel instead and start anew.

5.) Take screenshots faster

I think it would be better UX and more seamless if the "Swarm Screenshot" button in the system tray already started the timer without having to click "Take Screenshot" again.

The above observations were fixed in the latest push and as follows:

  1. License: 

  • Replaced copy.svg with in-house design via figma

  • Replaced audio with in-house modified audio from pixabay (royalty free)
  1. Fix negative file size
  2. Fix upload error with encrypt option
  3. Initiated capture on window load for seamless UX

@ferencsarai
Copy link
Member

Hi, great work!
At the bottom of the "screenshot Preview" window, the two buttons are not fully visible on macOS unless I scroll.
Screenshot 2024-11-26 at 10 42 52

@amochuko
Copy link
Contributor Author

Hi, great work! At the bottom of the "screenshot Preview" window, the two buttons are not fully visible on macOS unless I scroll. Screenshot 2024-11-26 at 10 42 52

Thank you.

In the latest push, I did a fix regarding each observation you made mention.

@ferencsarai
Copy link
Member

Thank you, great work! If the screenshot preview window could be made taller, that would be fantastic, as the buttons would be fully visible.

@amochuko
Copy link
Contributor Author

amochuko commented Nov 26, 2024

Thank you, great work! If the screenshot preview window could be made taller, that would be fantastic, as the buttons would be fully visible.

Thank you too.

I will go fix the height for that.

BTW, just to satisfy curiosity; my display is 13.3 inch Macbook and the height looks perfect.

Here are two snapshots:

  1. This is the default height.
Screenshot 2024-11-26 at 16 54 16
  1. The is the default height + 160 (latest push)
Screenshot 2024-11-26 at 17 08 11

@ferencsarai
Copy link
Member

Thank you for the screenshots. I use a lower resolution, which is why I encountered the issue. Apologies, the +160 increase is probably unnecessary. The suggested code modifications provide a good view for me at multiple resolutions (Default, More Space).

@amochuko
Copy link
Contributor Author

Thank you for the screenshots. I use a lower resolution, which is why I encountered the issue. Apologies, the +160 increase is probably unnecessary. The suggested code modifications provide a good view for me at multiple resolutions (Default, More Space).

Alright. I appreciate.

Please, just to clarify, should I revert back to initial default or we keep the +160?

@ferencsarai
Copy link
Member

Please revert to initial default.

@amochuko
Copy link
Contributor Author

Please revert to initial default.

Ok. I've reverted it.

const scaleFactor = primaryDisplay.scaleFactor

const defaultScreenSize = {
width: (width / resizeBy) * scaleFactor + 160,
Copy link
Member

Choose a reason for hiding this comment

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

width: Math.floor((width / resizeBy) * scaleFactor),
height: Math.floor((height / resizeBy) * scaleFactor),`

`

img {
display: block;
max-width: 90%;
max-height: 720px;
Copy link
Member

Choose a reason for hiding this comment

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

max-height: 640px;


previewWindow = new BrowserWindow({
width: defaultScreenSize.width,
height: defaultScreenSize.height,
Copy link
Member

Choose a reason for hiding this comment

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

useContentSize: true,

@ferencsarai ferencsarai merged commit d0a3906 into ethersphere:master Nov 28, 2024
1 of 2 checks passed
@amochuko amochuko deleted the screenshot-plugin branch November 28, 2024 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants