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

Link component should NOT prevent_default when command + clicked #2911

Closed
1 of 3 tasks
hoangph271 opened this issue Oct 7, 2022 · 1 comment · Fixed by #3056
Closed
1 of 3 tasks

Link component should NOT prevent_default when command + clicked #2911

hoangph271 opened this issue Oct 7, 2022 · 1 comment · Fixed by #3056
Labels
A-yew-router Area: The yew-router crate bug

Comments

@hoangph271
Copy link

hoangph271 commented Oct 7, 2022

Problem

On macOS, command + click on a link should be handled as open in a new tab.
The same behavior is expected for ctrl + click on Windows or Linux

The current implementation invokes the prevent_default() method on the event object without handling these odd cases.

Steps To Reproduce

Steps to reproduce the behavior:

  1. Create a Yew app with a <Link /> element from yew_router
  2. command + click on the link
  3. Observe the behavior: The link is opened in the current tab

Expected behavior
A new tab should be opened from the link

Environment:

  • yew = "0.19.3"
  • yew-router = "0.16.0"
  • Rust version: [1.64.0, stable]

Questionnaire

  • I'm interested in fixing this myself but don't know where to start
  • I would like to fix and I have a solution
  • I don't have time to fix this right now, but maybe later
@hoangph271 hoangph271 added the bug label Oct 7, 2022
@WorldSEnder
Copy link
Member

WorldSEnder commented Oct 7, 2022

If you want to write the PR for this, the place to start should be in the onclick handler here: https://github.com/yewstack/yew/blob/master/packages/yew-router/src/components/link.rs#L57-L58

Since the bug is about the control key specifically, https://rustwasm.github.io/wasm-bindgen/api/web_sys/struct.MouseEvent.html#method.ctrl_key should get you started to read that from the event, but you can think of other state that deserves handling, like shift, alt (or right-click? that seems to be handled at least in Firefox correctly) too.

@hoangph271 hoangph271 changed the title Link component should prevent_default when command + clicked Link component should NOT prevent_default when command + clicked Oct 7, 2022
@WorldSEnder WorldSEnder added the A-yew-router Area: The yew-router crate label Oct 8, 2022
kaisalmon added a commit to kaisalmon/yew that referenced this issue Dec 22, 2022
Prevents Link onclick behaviour from executing if the Ctrl key (for
Windows and Linux) or Meta Key (For Mac) is pressed.

This technically introduces a bug that means that links will reload the
page on windows machines when the windows key is held down. However,
this error is also in React Router, so I think we can get away with it.

See:
https://github.com/remix-run/react-router/blob/11156ac7f3d7c1c557c67cc449ecbf9bd5c6a4ca/packages/react-router-dom/dom.ts#L29
WorldSEnder pushed a commit that referenced this issue Dec 22, 2022
* Fixing #2911

Prevents Link onclick behaviour from executing if the Ctrl key (for
Windows and Linux) or Meta Key (For Mac) is pressed.

This technically introduces a bug that means that links will reload the
page on windows machines when the windows key is held down. However,
this error is also in React Router, so I think we can get away with it.

See:
https://github.com/remix-run/react-router/blob/11156ac7f3d7c1c557c67cc449ecbf9bd5c6a4ca/packages/react-router-dom/dom.ts#L29

* Router Links now use default browser behaviour for alt and shift keys.

This change is inspired by https://github.com/remix-run/react-router/blob/11156ac7f3d7c1c557c67cc449ecbf9bd5c6a4ca/packages/react-router-dom/dom.ts#L2://github.com/remix-run/react-router/blob/11156ac7f3d7c1c557c67cc449ecbf9bd5c6a4ca/packages/react-router-dom/dom.ts#L29

This allows uses to shift click links to save whatever the link points
at, and alt click on links to open them in new windows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew-router Area: The yew-router crate bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants