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

ctrl + click to open link in new tab not working #68

Closed
marvinhagemeister opened this issue Aug 4, 2019 · 4 comments
Closed

ctrl + click to open link in new tab not working #68

marvinhagemeister opened this issue Aug 4, 2019 · 4 comments

Comments

@marvinhagemeister
Copy link

For desktop browsers there are typical navigation patterns to open the link in a new tab instead of the current one. The most common one is ctrl + click or clicking with the middle mouse button. Currently this doesn't work because the event will always be prevented:

wouter/index.js

Lines 91 to 98 in c9c7900

const handleClick = useCallback(
event => {
event.preventDefault();
navigate(href);
onClick && onClick(event);
},
[href, onClick, navigate]
);

In preact-router we have this line which explicitly checks for common key combinations and lets the browser handle the event instead:

// ignore events the browser takes care of already:
if (e.ctrlKey || e.metaKey || e.altKey || e.shiftKey || e.button!==0) return;

https://github.com/preactjs/preact-router/blob/3eb5b31fe75c34672eeb13f2d310dee6d51b1f97/src/index.js#L112

@molefrog
Copy link
Owner

Hi @marvinhagemeister!
This is a really nice feedback, I'm a huge fan of ctrl+click myself, but somehow forgot about this scenario 😄

I've got two questions before applying the fix:

  1. Will this code be supported by React as well? Do I need to have an additional check to cover React?
  2. What does the e.button !== 0 check do?

@molefrog
Copy link
Owner

Ok, I jest checked and seems like it's going to be compatible with React, their SyntheticEvent has all these attributes.

And regarding 2: the check is needed in order to ignore all none-left button clicks. Seems like it should work!

@molefrog
Copy link
Owner

Done!
Fixed in 55fc794 and c54bcf2

@marvinhagemeister
Copy link
Author

@molefrog Holy moly that was quick! Happy to hear that you found the answers already 👍 Super excited about this router as this is the first one where I think "yep this is how a router should be done". Love it 💯

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

No branches or pull requests

2 participants