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

Fix/web context menu #3135

Merged
merged 2 commits into from
Oct 29, 2019
Merged

Fix/web context menu #3135

merged 2 commits into from
Oct 29, 2019

Conversation

hackily
Copy link
Contributor

@hackily hackily commented Oct 29, 2019

PR Checklist

Please check all that apply to this PR using "x":

  • I have checked that this PR is not a duplicate of an existing PR (open, closed or merged)
  • I have checked that this PR does not introduce a breaking change
  • This PR introduces breaking changes and I have provided a detailed explanation below

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting)
  • Refactoring (no functional changes)
  • Documentation changes
  • Other - Please describe:

Fixes

If the context is outside of the electron app, the context menu will not be handled.

Issue Number: #3065

What is the current behavior?

Right clicking will bring up a context menu in the elctron app within lbry.tv
Right clicking in a browser on content will not bring up the context menu, and will print errors in the console.

What is the new behavior?

Electron app continues to behave as normal.
Native context menu now appears in browsers.

Other information

import { clipboard, remote, isDev } from 'web/stubs'; // eslint-disable-line

Would have loved to be able to include the import clipboard on line 6 in /ui/util/context-menu.js, but it seems that despite attempting to add an eslint-disable-line comment, eslint will return this anyways:

4 | // @endif
5 | // @if TARGET='web'
> 6 | import { clipboard, remote, isDev } from 'web/stubs'; // eslint-disable-line
  |          ^
7 | // @endif
8 |
9 | function injectDevelopmentTemplate(event, templates) {   

✖ 1 problem (1 error, 0 warnings)

@tzarebczan
Copy link
Contributor

@hackily , thank you and congrats on the first contribution! We'll get a review on this soon.

Can we show you some appreciation for the contribution?

Also, we're giving away some Hacktoberfest bonuses and goodies for this month.

@neb-b neb-b merged commit 209ee7d into lbryio:master Oct 29, 2019
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.

None yet

3 participants