Skip to content

Feature: Screenshot copy to clipboard#9042

Open
Shadorc wants to merge 26 commits into
FreeTubeApp:developmentfrom
Shadorc:feat/screenshot-clipboard
Open

Feature: Screenshot copy to clipboard#9042
Shadorc wants to merge 26 commits into
FreeTubeApp:developmentfrom
Shadorc:feat/screenshot-clipboard

Conversation

@Shadorc
Copy link
Copy Markdown
Contributor

@Shadorc Shadorc commented Apr 20, 2026

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

closes #8641

Description

  • Replace "Ask for Save Folder" toggle with a dropdown allowing to chose a new option: copy to clipboard

Screenshots

After:
image
image
image
image

Testing

Copy to Clipboard:

  1. Select "Screenshot Mode: Copy to Clipboard"
  2. Verify that only all screenshot options are either invisible or removed
  3. Verify that the screenshot button in the player is present
  4. Verify that clicking on the screenshot button copy the screenshot to clipboard and does not save on disk

Desktop

  • OS: Bazzite (Fedora based Linux distro)
  • OS Version: 43
  • FreeTube version: 0.24.0

Additional context

  • I'm not really satisfied of the "both" option, not really future proof if more modes are added. It could be two toggles, but again not really sure what the UI would look likle with more modes. Let me know what you think.
  • Electron Clipboard API only supports JPEG and PNG but from my tests it seems that ultimately the image ends up as PNG anyways
  • This feature is breaking backward compatibility as the "enable screenshot" option is gone. Not sure if there are migrations code or if retrocoompatibility should be kept no matter what.

I'm new to this project so I still have a bit to learn about!

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) April 20, 2026 23:14
@github-actions github-actions Bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Apr 20, 2026
@absidue
Copy link
Copy Markdown
Member

absidue commented Apr 26, 2026

Please use the standard web navigator.clipboard API, like we do for copying text, rather than going to the main process and using the Electron specific APIs. Copying to the clipboard should be possible in FreeTubeAndroid too just like prompting for a save location (only the save to default folder is electron only).

I agree with you that "both" is odd, please remove it. Additionally rather than replacing the enable screenshot switch it would be better if you replaced the prompt for save location one. That dropdown would still default to prompting for the save location, in Electron builds it should have 3 options (the two existing ones plus copying to the clipboard) outside of Electron e.g. in FreeTubeAndroid it should only have prompting for the save location or copying to the clipboard.

As you already discovered it only makes sense to write PNGs to the clipboard, that id because images sit on the clipboard in operating system specific bitmap formats, not PNGs or JPEGs, so writing a PNG avoids losing details that would be lost in the lossy conversion to JPEG. As copying to the clipboard will always use PNG, the format and quality options should be hidden or at least disabled when the clipboard option is selected.

Testing an actual FreeTubeAndroid build is a bit more involved, so you can check the settings UI outside of Electron with pnpm run dev:web, that is unusable for most things as FreeTube cannot fetch information from YouTube in a normal web browser but for "offline" stuff like the settings page it should work fine.

@absidue absidue added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Apr 26, 2026
auto-merge was automatically disabled April 26, 2026 23:50

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) April 26, 2026 23:50
auto-merge was automatically disabled April 27, 2026 00:11

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) April 27, 2026 00:11
@Shadorc
Copy link
Copy Markdown
Contributor Author

Shadorc commented Apr 27, 2026

Thanks for your review!

I have applied the requested changes and updated my initial messsage accordingly.
I was also able to test in browser to simulate USING_ELECTRON

auto-merge was automatically disabled April 27, 2026 00:24

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) April 27, 2026 00:24
Comment thread static/locales/en-US.yaml Outdated
auto-merge was automatically disabled May 1, 2026 20:00

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) May 1, 2026 20:01
Copy link
Copy Markdown
Member

@absidue absidue left a comment

Choose a reason for hiding this comment

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

In addition to the other two comments, could you please also add a migration from the old setting to the new one in src/datastore/handlers/base.js, you could take inspiration from the other setting migrations that are already there.

Comment thread static/locales/en-US.yaml Outdated
Comment thread src/renderer/helpers/utils.js Outdated
auto-merge was automatically disabled May 2, 2026 18:24

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) May 2, 2026 18:25
auto-merge was automatically disabled May 9, 2026 16:11

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) May 9, 2026 16:12
@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Copy Markdown
Member

I've also hidden screenshot options when clipboard is selected because it has no meaning

You've somehow read my mind :O, thanks for doing this. PR LGTM, hope to see more from you in the future :)

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels May 10, 2026
@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc requested review from PikachuEXE and absidue and removed request for absidue May 10, 2026 09:51
Copy link
Copy Markdown
Member

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Only the middle one is missing the action target

Image

Local preview of proposed change
Image

If you want it to look better especially when select closed just add class name and add style to it

Image Image Image Image

Comment thread static/locales/en-US.yaml Outdated
Comment thread src/renderer/components/PlayerSettings/PlayerSettings.vue Outdated
auto-merge was automatically disabled May 12, 2026 10:40

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) May 12, 2026 10:40
@Shadorc
Copy link
Copy Markdown
Contributor Author

Shadorc commented May 12, 2026

@PikachuEXE Thanks for your suggestion. I've used inline-size: fit-content; instead, if that's ok for you, to support long localized texts

@PikachuEXE
Copy link
Copy Markdown
Member

PikachuEXE commented May 13, 2026

The spacing doesn't look right
image

If padding removed, it looks wrong when longer options selected
image

image

Edit 1: It submitted before I can paste all images...

Also I forgot I need to check narrow screen view...
image

@PikachuEXE
Copy link
Copy Markdown
Member

PikachuEXE commented May 13, 2026

Well maybe just update label text to Save to Folder and we don't have to mess with the style (no idea why I didn't think of this before

Edit: local preview
image
image
image

@Shadorc
Copy link
Copy Markdown
Contributor Author

Shadorc commented May 13, 2026

I've scratched my head at this problem because I really wanted the content to fit the select for localization. I discovered that the correct solution would be

.screenshotModeSelect {
  min-inline-size: fit-content;
}

.screenshotModeSelect .select-text {
  margin-right: 15px;
}

but .select-text is scoped, I cannot add an attribute outside of the FtSelect.css file.
I don't think I can fix this easily without messing with FtSelect, so I will just apply your short text suggestion.

Thank you!

auto-merge was automatically disabled May 13, 2026 08:37

Head branch was pushed to by a user without write access

@Shadorc
Copy link
Copy Markdown
Contributor Author

Shadorc commented May 13, 2026

Results:
image
image

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) May 13, 2026 08:37
PikachuEXE
PikachuEXE previously approved these changes May 14, 2026
`find static/locales/ -name '*.yaml' -exec sed -i 's/Ask Path:/Modes:\n        Ask Path:/' {} \;`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: waiting for review For PRs that are complete, tested, and ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request]: Save screenshot to clipboard

4 participants