Skip to content
This repository has been archived by the owner on Sep 1, 2020. It is now read-only.

[Feature] Prevent context click from changing document.activeElement #320

Closed
Nantris opened this issue Mar 30, 2020 · 20 comments
Closed

[Feature] Prevent context click from changing document.activeElement #320

Nantris opened this issue Mar 30, 2020 · 20 comments

Comments

@Nantris
Copy link

Nantris commented Mar 30, 2020

It would be great to avoid taking focus when the user right clicks the <ContextMenuTrigger />

I know this is probably impossible to achieve with the longpress action, but it would great if it were the case for right clicks.

@vkbansal
Copy link
Owner

Can you open a PR with the intended changes?

@Nantris
Copy link
Author

Nantris commented Mar 31, 2020

Not sure I'll ever find the time, but knowing that you're open to the PR I will at least try to. Thanks for your response!

@manoharreddyporeddy
Copy link

manoharreddyporeddy commented Apr 8, 2020

@vkbansal @slapbox

Expected - For example, right click on this text, and select copy -- the initially selected text remains selected in the browser.

Actual - When i right click on text where 'react-contextmenu' is activated, and selecting an option from the context menu, my initial selection is lost.

  • How can this be enabled, via an option to 'react-contextmenu'?
  • Or, which code files to edit so I can give a PR for this?

Is this related to this bug, should I raise another?

Thank you

@Nantris
Copy link
Author

Nantris commented Apr 8, 2020

@manoharreddyporeddy I believe event.preventDefault() is needed in here https://github.com/vkbansal/react-contextmenu/blob/master/src/ContextMenuTrigger.js#L38-L49

Preventing the focus from changing should be pretty easy, but will prevent users from using holdToDisplay to open the menu.

@manoharreddyporeddy
Copy link

Thanks for reply @slapbox
@vkbansal

A - scenario
I actually have to select multi word text, and select a context menu option
for the same selection again, I have to select another context menu option
Now, this is not possible if previous selection is not persistent.

B. May be bug?
holdToDisplay is already set to -1
However, this issue remains.
May be event.preventDefault() should already be applied?

C. How to build?
thanks for showing the place to change to change
I could not find doc to build this app
can u help on - how to build the app library?

@vkbansal
Copy link
Owner

vkbansal commented Apr 9, 2020

For building you can just npm run build

see https://github.com/vkbansal/react-contextmenu/blob/master/package.json#L28

@Nantris
Copy link
Author

Nantris commented Apr 9, 2020

@manoharreddyporeddy, event.preventDefault() in onMouseDown handler should prevent your text area from losing both focus and the current selection. If it's not, then some other logic is changing the element focus.

For testing's sake, what happens if you add event.preventDefault() here, before the if statement? Does this properly prevent the loss of focus?

If no, what if you use:

event.preventDefault();
return;

Obviously this will break the context menu completely, but it should not steal focus from your text area. If it still steals the focus, something weird is going on. Let me know how it goes!

@manoharreddyporeddy
Copy link

manoharreddyporeddy commented Apr 10, 2020

@slapbox @vkbansal

Thanks for trying to help.

npm run build dint create the dist directroy
npm run build:dist did create directory, but to use react-contextmenu.js I had to add /* eslint-disable */ at the begin of the file

Then already added above lines in various places in the same function.
After the suggestion give above was made, I was not able to select the text at all, which i dint want.

After trying this for 2 days, I am trying to not use the library - but unfortunately I have written a lot of code for this already - with demo time coming soon I am in a bit of problem trusting this library to take care of things.

I would like to ask one question:

  • To build this context menu from scratch, using plain javascript, what are the resources you can give ? both links (external or internal to this library) & api (mdn or other) - no time for debugging this library for now sorry - quick help appreciated

Thank you

@Nantris
Copy link
Author

Nantris commented Apr 10, 2020

@manoharreddyporeddy I spent probably a bit over an hour reviewing the available context menus for React. I'd strongly recommend focusing on solving the issue on this library rather than writing an entire new library. The solution should be pretty much as simple as event.preventDefault() in the right spot.

Did you try the two troubleshooting tips I suggested above? If you let me know the outcome of those two things that would be a big help in figuring out the problem. Unfortunately I don't have any advice whatsoever for building a context menu from scratch.

@manoharreddyporeddy
Copy link

@slapbox

I already answered your questions:

already added above lines in various places in the same function.
After the suggestion give above was made, I was not able to select the text at all, which i dint want.

Thanks for reply on unable to give advice on from scratch.

@vkbansal

@Nantris
Copy link
Author

Nantris commented Apr 11, 2020

@manoharreddyporeddy can you visually demonstrate what you're trying to achieve? I'm starting to question whether I understand properly.

Regarding how to make a context menu on your own, start with PopperJS or React-Popper (based on PopperJS).

@vkbansal
Copy link
Owner

Thanks for reply on unable to give advice on from scratch.

@manoharreddyporeddy I, like you, have a full-time job, family and responsibilities. I'm sorry that I was not able to help you. If you are not happy with this project, please go ahead and use some other project.

You must understand that 90% of such open-source projects are maintained by single person team, who take out some time from their free/family time to help others.

Here are some links for other packages that you can use:
https://www.telerik.com/kendo-react-ui/components/layout/menu/context-menu/
https://blueprintjs.com/docs/#core/components/context-menu
https://www.npmjs.com/search?q=contextmenu

@manoharreddyporeddy
Copy link

manoharreddyporeddy commented Apr 11, 2020

@slapbox

I have previously give the scenario already:

A - scenario
I actually have to select multi word text, and select a context menu option
for the same selection again, I have to select another context menu option
Now, this is not possible if previous selection is not persistent.

That means,
I have some text - that is wrapped in this libray
** I select for example, 3 words
when I right click, on the selected text, a context menu appears
when i select an option in the context menu - the selection made previous (** above) disappears
That should not happen because, I should be able to right click and get context menu again, then select another option

In other words, i need the text selected + 2 options from context menu I selected for the selected text (from 2 right clicks)

this is same as
if you have right clicked anywhere on browser text and selected "Copy" -- text selection remains
then again right clicked on same text and selected "Copy2" (assuming we have Copy 2 option)

The "text selection remains" is not happening with this library.

Since you din't ask a question, my assumption was you understood, it cost another day.
Please ask question if you din't understand - 2 time read of above you should be able to understand.

@manoharreddyporeddy
Copy link

manoharreddyporeddy commented Apr 11, 2020

@vkbansal

@manoharreddyporeddy I, like you, have a full-time job, family and responsibilities. I'm sorry that I was not able to help you. If you are not happy with this project, please go ahead and use some other project.
You must understand that 90% of such open-source projects are maintained by single person team, who take out some time from their free/family time to help others.

It is obvious that open source is like that - single or some more person jobs
so I did not expect immediate action from you
but expected, at least you would hear & learn your user's problems
I did not expect a reply like the you did now
because it did not help recover my (your library's users) lost time, your kind of reply only added a bit of strain now on already strained user.

@Nantris
Copy link
Author

Nantris commented Apr 11, 2020

2 time read of above you should be able to understand.

@manoharreddyporeddy obviously English is not your first language. Your responses seem kind of rude. I'm sure you're stressed out, but when people try to help you, and when you are not a native speaker, you should not fault the reader for not understanding. I did ask a question, as soon as you wrote something that gave me reason to think I didn't understand.

I'm going to assume though that you don't mean to be rude, and that this is just a language barrier thing, so I have poked around a bit further.

The issue I have is that I lose focus when I click to open the context menu. It sounds like you're not losing focus until you actually click one of the menu items. If that's the case, the relevant code appears to be here:

https://github.com/vkbansal/react-contextmenu/blob/master/src/MenuItem.js#L78-L87

Add an onMouseDown=(event => event.preventDefault()} to that element and let me know how it goes.

@vkbansal
Copy link
Owner

@manoharreddyporeddy I think I did not say anything wrong here. I just pointed you to alternatives if this library is not a good solution for a problem.

@stale
Copy link

stale bot commented Jun 12, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 12, 2020
@Nantris
Copy link
Author

Nantris commented Jun 13, 2020

Not stale. @vkbansal is there a way to prevent stalebot from wontfixing this?

@stale stale bot removed the wontfix label Jun 13, 2020
@vkbansal
Copy link
Owner

I'll into removing stalebot.

@vkbansal
Copy link
Owner

closing issue see #339

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants