[🐞] SPA routing is preventing browser defaults

[tzdesign]

Which component is affected?

Qwik Runtime

Describe the bug

When you press cmd + click on a link, qwik uses preventDefault.

The default browser experience would open a new tab. If you add shift to the party, it'll even open it directly. I know there are several other shortcuts to open in a new window or private window.

Qwik should not prevent that kind of functionality.

Reproduction

https://stackblitz.com/edit/qwik-starter-qads2x?file=src%2Froutes%2Findex.tsx

Steps to reproduce

Run it on stackblitz.

Click flower app with cmd

You just get routed

Click todo app with cmd

You get to a new tab

System Info

See stackblitz

Additional Information

No response

[bolt-new-by-stackblitz[bot]]

Start a new pull request in StackBlitz Codeflow.

• qwik-starter-qads2x (Open reproduction)

[manucorporat]

is it a thing in other metaframeworks like nextjs, when using Link? sounds more like a feature request than a bug

[tzdesign]

@manucorporat no I think you are preventing default. In react-router and next it's only preventing default if shiftKey, meta and ctrl are not true.

I checked it with our internal apps and my personal nextjs page.

We discussed this with @hamatoyogi and @shairez, and I think we agreed that qwik should avoid preventing browser APIs as much as possible.

You can call it a feature request, but I would say it should have been implemented in the same way already.

[tzdesign]

@jordanw66 I saw you changes on the link component #4558

The current component completely suppresses the default from the browser. Examples:

• Meta + Click / Ctrl + Click » Open new Tab
• Alt + Click » Download resource
• Meta + Shift + Click » Open Tab in foreground

To get this to work, the only thing I can think of is to toggle preventDefault in the event. But as it's async it's impossible to get it to work.

Do you have any idea, if this is even possible? @manucorporat said there is currently no way to dynamically preventDefault conditionally.

I was thinking of smt like this, but nativeEvent is empty:

const isClientNavigation =
        (event.metaKey || event.altKey || event.ctrlKey || event.shiftKey) === false &&
        Boolean(clientNavPath);
      if (isClientNavigation === false) {
        // Do not enter the nav pipeline if this is not a clientNavPath or the user hit one of the modifier keys.
        return;
      }
      event.nativeEvent?.preventDefault();

[tzdesign]

@jordanw66 I agree, trying to fake the default behavior is not an option.

I think that qwik suppresses the default behavior here is a mistake. However, I don't know how important it is to everyone to go to the qwik-loader here.

Do you think it's justified to pursue it here? I personally use meta + shift + click every single day.

@manucorporat what is your opinion about it?

[tzdesign]

@jordanw66 perfect idea. Your suggestion is awesome. The question is, how deep do we want to go. I might want to implement it for all developers and pass the allowmodifiers as a type like that:

export type AllowModifiers = SingleOrArray<
  keyof Pick<QwikMouseEvent, 'altKey' | 'ctrlKey' | 'metaKey' | 'shiftKey'>
>;

This way it's connected to the event and the developer can be specific. For link we set linkProps to all of them.

I have some space tomorrow and can check it out, but maybe @manucorporat can tell us if this is ok. We also can catch up and talk about it, as Manu wanted to write e2e tests for navigation?

[tzdesign]

is it a thing in other metaframeworks like nextjs, when using Link? sounds more like a feature request than a bug

https://github.com/vercel/next.js/blob/e33b87d894a8e80a4328adf1fd3d472896935be0/packages/next/src/client/link.tsx#L208

function isModifiedEvent(event: React.MouseEvent): boolean {
  const eventTarget = event.currentTarget as HTMLAnchorElement | SVGAElement
  const target = eventTarget.getAttribute('target')
  return (
    (target && target !== '_self') ||
    event.metaKey ||
    event.ctrlKey ||
    event.shiftKey ||
    event.altKey || // triggers resource download
    (event.nativeEvent && event.nativeEvent.which === 2)
  )
}

@manucorporat here is how next is doing it, but of course they are only on the client in this case.

[gioboa]

This is solved by sync$()

[tzdesign]

@gioboa @mhevery just perfect 🥹

The sync$ is also a pretty awesome new feature

[gioboa]

Nice, I'm glad to hear that 😉