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

Add right click context menu on all text inputs #1486

Merged
merged 3 commits into from May 29, 2018

Conversation

Projects
None yet
3 participants
@dan1d
Copy link
Contributor

dan1d commented May 16, 2018

Hi this PR partially fix: #1470, this pr adds the right click menu only on text inputs and copy/cut/paste works, also if you are in development mode it will show "open developers tools".

So currently works on this two items:
1.) if the copy cut paste will be enforced in the search bar then i'd recommend a "select all" option as well.
3.) If there is nothing in the search, the cut and copy can be greyed out leaving the paste option visible if there is something on the clipboard otherwise also greyed.
screenshot from 2018-05-16 18-54-43
screenshot from 2018-05-16 18-35-57

On this one:
2.) A right click on any of the videos can give options like copy video url or any other information on the video that copy cut paste.
I need to figure out what's the best approach to fix this one, if you have an idea, let me know!

@seanyesmunt

This comment has been minimized.

Copy link
Member

seanyesmunt commented May 16, 2018

Thanks for the PR @dan1d! We already have some code for the context menu here: https://github.com/lbryio/lbry-app/blob/master/src/main/menu/setupContextMenu.js

That would be a better place to add this.

@dan1d

This comment has been minimized.

Copy link
Contributor Author

dan1d commented May 21, 2018

Hi @seanyesmunt ,
After looking around on others libraries that handlesContext and reading the electron documentation, it took me a lot of testing/moving/copiying/looking other people solutions/react context menu libraries, I think I have the easiest and correct solution, I've realized that we can call the function onContextMenu on any component(if I knew this before It would saved me a lot of reading on github issues)

I know you suggested on discord to move it to renderer/ folder, but I think the context menu shouldn't be a component as It can be easily modified.
I've deleted the code under src/menu/setupContextMenu.js and moved it to util/contextMenu which has a few functions exported to open the desired menu, if it is an input then it open this one:
screenshot from 2018-05-20 22-12-26

If it is a video link, the react component can be customized to open a different context menu:

screenshot from 2018-05-20 22-08-31
Currently when I copy the link I get an url copied like this one: lbry://why-i-m-bullish-but-selling-alts#19d0d7e3a3c17303d4b69fcc2b78512dd58c74db, how I should build the url to be able to share it to others users and when they click the link the lbry-app is the one that opens that link?, is there a service to use ?

Also, could you take a look at the new code on this PR please?
Also I can add a snackbar notification when the link is copied if it is required, I've only added the copy link feature just to the home page but it can be added on a lot of others components.

The code is also development aware so it will only show the inspect element option only if the environment is DEV.

Daniel

@dan1d

This comment has been minimized.

Copy link
Contributor Author

dan1d commented May 21, 2018

Hi @seanyesmunt I've updated the code on this PR to parse an URI and added a new PR on lbry-redux, read my PR comment on lbry-redux!

So for example given this URI:
lbry://why-doesn-t-capitalism-work-for#36cd5b217bcc4083a6fc35cc4de2cc1196581ef9
it will convert it to this
https://open.lbry.io/why-doesn-t-capitalism-work-for#36cd5b217bcc4083a6fc35cc4de2cc1196581ef9
and now the link can be shared by just copying the link when right click is pressed on a video card.

@tzarebczan tzarebczan requested a review from seanyesmunt May 21, 2018

@lbry-bot lbry-bot assigned seanyesmunt and unassigned seanyesmunt May 21, 2018

@seanyesmunt

This comment has been minimized.

Copy link
Member

seanyesmunt commented May 22, 2018

@seanyesmunt seanyesmunt merged commit 69c0303 into lbryio:master May 29, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.