-
Notifications
You must be signed in to change notification settings - Fork 90
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
base: develop
Are you sure you want to change the base?
Conversation
@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. |
I looked into making an e2e test for this, but it isn't a great candidate, I don't think. Selenium could right click on an element, but then it could not see or manipulate the context menu, probably because it is in the Electron chrome and not in the page itself. There are some articles (e.g. https://elementalselenium.com/tips/63-right-click) that say you can manipulate the context menu with arrow keys and perhaps use keys for copy/paste, but this didn't work either. I did learn about e2e tests, get the linux ones running locally, and update |
Adds a context menu in Quiet desktop for copying text (#503) and saving images (#568)
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