- Notifications
You must be signed in to change notification settings - Fork2k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
docs: consider ctrl clicking an anchor#12290
Conversation
|
| @@ -59,7 +59,7 @@ For this to work, you need to load the data that the `+page.svelte` expects. A c | |||
| href="/photos/{thumbnail.id}" | |||
| on:click={async (e) => { | |||
| // bail if opening a new tab, or we're on too small a screen | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It might be nice to expand this with some details about ctrl/meta on various OS to explain why we need to check both
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
DoesWindows + clicking a link open it in a new tab?
Cannot reproduce it in Windows 11 + Chrome | Firefox.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It’s ctrl + click to open in a new tab on windows iirc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Windows+click (for me, on Firefox) on a regular link does nothing - it doesn't follow it at all. It looks like itdoes trigger aclick event, though, so to match that behavior, we should also make sure that we bail here and let the browser use the default handler, which is to do nothing.
However, there are also a number of other cases we should be handling here besides meta-click. Shift-click opens in a new window, and so we want to use the default handling there. Alt-clicking (for me) does nothing, so we also want the default handling there. I'm uncertain whether we need to check that this was a left-button click.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I believe a scroll-wheel clicking on an anchor also opens it in a new tab as well. Should this be handled as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
adding support for scroll-wheel clicking would be a nice touch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Scroll-wheel click seems to be checkable usinge.button === 1.
Updated theREPL to log the mouse button used to click the anchor.
Referencehttps://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/button
Create a SvelteREPL just to make sure. To open an anchor in a new tab:
|
Thanks! Linux is also |
Maybe This is what Instagram is doing. In a Instagram profile, There is always on option to open a IG post in a new tab by using the context menu. |
I'd be fine with that. This is just sample code, so I don't really want to over-complicate it by handling all the various cases (#12290 (comment)) |
On the other hand, I think people likelyare going to take inspiration from this code, and will likely lift parts of it directly - I don't think we want to assume that everyone is going to do their own research into which click events should be left to the browser. There are a number of obscure cases, as demonstrated in this discussion. |
yeah, that's true. I suppose even if they're not building an Instagram clone, they'll still copy/paste parts of this |
| @@ -59,7 +59,7 @@ For this to work, you need to load the data that the `+page.svelte` expects. A c | |||
| href="/photos/{thumbnail.id}" | |||
| on:click={async (e) => { | |||
| // bail if opening a new tab, or we're on too small a screen | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| // bail if opening a new tab, or we're on too small a screen | |
| // bail if opening a new tab, or we're on too small a screen | |
| // to modify a link click to open in a new tab, macOS uses metaKey and windows/linux use ctrlKey |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Since Chrome uses the 'open link in new tab' context menu text, how about the following?
- for clicking to open a new tab,+ for opening a link in a new tab,+ to open the clicked link in a new tab,``
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I tweaked the suggestion a bit. what do you think about it now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That is intuitive. Can I add 'the' in-front of the key names?
macOS uses the metaKey while windows and linux use the ctrlKey
Following citation is from Wikipedia.
Similarly to theShift key, the Control key rarely performs any function when pressed by itself. The Control key is located on or near the bottom left side of most keyboards (in accordance with the international standardISO/IEC 9995-2), with many featuring an additional one at the bottom right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can I add 'the' in-front of the key names?
It does read nicely with 'the', but I was kind of trying to keep it on one line. We might have to break it into two lines if it gets too long, so I don't know if it'd be worth it or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I personally think splitting each checks is more readable.
// bail if the user wants to open the link in a new tab// macOS uses the metaKey, windows/linux use the ctrlKeyif(e.metaKey||e.ctrlKey)return;// bail if the user wants to open the link in a new windowif(e.shiftKey)return;// bail if the screen is too narrow/smallif(innerWidth<640)return;
It might not be the most concise code, but there are more checks to add.
Will be adding this commit for now (also sincee.metaKey has to be reverted)
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
thanks! this was surprisingly more complicated than i'd have imagined, but glad we figured something out |
On Windows,
Ctrlclicking an anchor opens it in a new tab.Checking for
e.metaKeyonly checks for the Windows key.Originally suggested inHuntabyte's video starting at 6:00.
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits