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: update link component to allow modifier+click to properly work, and write tests #534

Merged
merged 6 commits into from
Feb 9, 2025

Conversation

jacobhq
Copy link
Member

@jacobhq jacobhq commented Feb 8, 2025

Checklist

Related issue

Fixes #531

Overview

This PR makes the link fallback to default a behaviour when a modifier key is being pressed.

@github-actions github-actions bot added the typescript Requires typescript knowledge label Feb 8, 2025
@jacobhq
Copy link
Member Author

jacobhq commented Feb 8, 2025

To test in docs, replace tuono version in package.json with file:../../packages/tuono after building, then do pnpm install.

2025-02-08.15-46-52.mp4

@jacobhq jacobhq marked this pull request as ready for review February 8, 2025 15:48
Copy link
Member

@Valerioageno Valerioageno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. I think you are on the right track!
I think the time has come for adding also an integration test for the Link component. Too many conditions.
Do you mind adding them within this PR? Let me know if you need help

@jacobhq
Copy link
Member Author

jacobhq commented Feb 8, 2025

I think the time has come for adding also an integration test for the Link component. Too many conditions.

Agreed, I'm happy to do that!

Copy link
Member

@marcalexiei marcalexiei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now all links now open in a new tab, with or without modifier key pressed.

This behaviour is caused by the code of MdxLink:

<Anchor
component={Link}
{...props}
target="_blank"
variant="transparent"

This issue was hidden by the event.preventDefault always executed on Link click event handler.
The target="_blank" should be removed from MdxLink component when rendering internal links.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Feb 9, 2025
@jacobhq jacobhq requested a review from marcalexiei February 9, 2025 10:43
marcalexiei

This comment was marked as resolved.

@jacobhq jacobhq changed the title fix: update link component to allow Ctrl/Cmd+Click to open in new tab fix: update link component to allow modifier+click to properly work, and write tests Feb 9, 2025
@jacobhq jacobhq requested a review from marcalexiei February 9, 2025 14:34
@marcalexiei marcalexiei merged commit 253f35d into tuono-labs:main Feb 9, 2025
18 checks passed
@jacobhq jacobhq deleted the jm-docs-internal-links branch February 9, 2025 19:32
@jacobhq jacobhq restored the jm-docs-internal-links branch February 22, 2025 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation typescript Requires typescript knowledge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Link leading to other documentation pages aren't opened in a new tab using cmd + click
3 participants