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

Update e.$slug.$theme.tsx #288

Closed
wants to merge 1 commit into from

Conversation

BrancuAlexandru
Copy link

added blank target to the vscode web link. It's super annoying to have to go back to the vscode themes page after trying each theme

@jschr
Copy link
Member

jschr commented Jan 14, 2025

Hey, thanks for the PR. I'm not a fan of changing the default behaviour for links. My thought was people would just use cmd (or ctrl) + click to open in a new tab if they want.

I don't think there is a technical reason to do this. If others disagree then I may reopen.

https://css-tricks.com/use-target_blank/
https://stackoverflow.com/questions/946248/when-should-you-use-target-blank-on-your-links

@jschr jschr closed this Jan 14, 2025
@jschr jschr mentioned this pull request Jan 14, 2025
@BrancuAlexandru
Copy link
Author

My thought was that it's a website where you are browsing for something, checking out multiple items. The default behavior should be one that best suits this purpose. It's possible to ctrl + click or middle mouse btn click, but that's not really the expected behavior.

I should've opened an issue about it before committing so it will come up for other people. Anyway, have a nice day, I'll make a user script for myself to add that attribute lol.

@jschr
Copy link
Member

jschr commented Jan 14, 2025

Thanks. I've opened an issue here. Hopefully you can understand the hesitation to change the default behaviour but I would love to hear from others on the topic as well.

@BrancuAlexandru
Copy link
Author

Yea, I completely understand.

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

Successfully merging this pull request may close these issues.

2 participants