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

Adds desktop context menu (#503, #568) #2719

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

holmesworcester
Copy link
Contributor

@holmesworcester holmesworcester commented Feb 1, 2025

Adds a context menu in Quiet desktop for copying text (#503) and saving images (#503)

This adds a small tweak from an earlier PR (#2355) by @agiledev24 to keep the new change from breaking unit tests.

Thank you @agiledev24 for your contribution!

Pull Request Checklist

  • I have linked this PR to a related GitHub issue.
  • I have added a description of the change (and Github issue number, if any) to the root CHANGELOG.md.

@holmesworcester
Copy link
Contributor Author

@islathehut -- this fails dependency review, but the dependency seems to be used by a lot of electron-related packages, not the one added by this PR. how do you suggest I proceed?

Also, it looked like it could be tricky to include the context-menu in the unit tests, and that the value could be limited, since this seems like a "set and forget" feature that will not interact much with other features--at least not until we start using the context menu ourselves for our own particular functions.

It also seemed like this was not the best place to learn about mocks and tests, but maybe I'm wrong about that.

What do you think? Want to make me write a test for this?

@islathehut
Copy link
Collaborator

@islathehut -- this fails dependency review, but the dependency seems to be used by a lot of electron-related packages, not the one added by this PR. how do you suggest I proceed?

Also, it looked like it could be tricky to include the context-menu in the unit tests, and that the value could be limited, since this seems like a "set and forget" feature that will not interact much with other features--at least not until we start using the context menu ourselves for our own particular functions.

It also seemed like this was not the best place to learn about mocks and tests, but maybe I'm wrong about that.

What do you think? Want to make me write a test for this?

@holmesworcester I wouldn't worry about the dependency review, especially if its on electron since we're not gonna be upgrading that right now.

For tests this may be a good candidate for e2e tests.

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