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

Made link open a new tab #221

Merged
merged 1 commit into from Aug 17, 2021
Merged

Made link open a new tab #221

merged 1 commit into from Aug 17, 2021

Conversation

Electogenius
Copy link

I wouldn’t want my code to disappear when I want to, say, look up a command or star the repo.

Note that I haven’t tested if this works yet but it probably works

I wouldn’t want my code to disappear when I want to, say, look up a command or star the repo
@ysthakur
Copy link
Member

Sounds like a good idea to me.

Copy link
Member

@vyxal-cookie vyxal-cookie left a comment

Choose a reason for hiding this comment

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

cookie

@ysthakur ysthakur requested a review from pxeger August 17, 2021 18:11
@ysthakur
Copy link
Member

@vyxal-cookie May I take your review as a serious one and merge this? I have no idea who you are but I'm assuming you're one of the actual contributors.

@Electogenius
Copy link
Author

Thanks!

Why’d the docs update fail? I don’t think adding an html attribute interferes with building docs, it says there was a 403 error fetching the git thingy.

(Also yes I don’t know who vyxal-cookie is 🤷‍♂️)

@ysthakur
Copy link
Member

ysthakur commented Aug 17, 2021

@Electogenius Don't worry about that, it's just because the Update docs workflow pushes to master and it can't do that right now because it's a different branch. It always fails on PRs. (I shouldn't have run the workflows in the first place, there was nothing to test)

As for vyxal-cookie, we're not sure which one of us that is. It's probably not lyxal or hyper-neutrino, and pxeger claims it's not his.

@@ -196,7 +196,7 @@
<body onload="initCodeMirror(); decodeURL(); updateCount()">
<textarea id=dummy style="visibility: hidden;position:absolute; height: 0px; font: size 20px;"></textarea>
<session-code>{{session}}</session-code>
<h2 style="display: inline-block;"><a href="https://github.com/Vyxal/Vyxal">Vyxal</a></h2>
<h2 style="display: inline-block;"><a href="https://github.com/Vyxal/Vyxal" onclick="window.open('https://github.com/Vyxal/Vyxal')">Vyxal</a></h2>
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's wrong with <a ... target="_blank">...</a>?

Copy link
Member

Choose a reason for hiding this comment

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

That's what I found on SO too, but this works, so ¯\(ツ)/¯.

ysthakur added a commit that referenced this pull request Aug 17, 2021
pxeger [pointed out](#221 (comment)) that `window.open` was unnecessary.
@Electogenius
Copy link
Author

Thanks! I didn’t know target="_blank" was a thing (though i’d seen it before) so I guess I just learned something new hehe

@Electogenius Electogenius deleted the patch-1 branch August 18, 2021 07:27
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.

None yet

4 participants