fix(angular): routerLink allows opening in a new tab for link elements

[sean-perkins]

Pull request checklist

Please check if your PR fulfills the following requirements:

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
• Build (npm run build) was run locally and any changes were pushed
• Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe):

What is the current behavior?

Users are unable to ctrl/CMD + click on link elements (<a routerLink="/">) to open a link in a new tab.

Issue URL: #24413

What is the new behavior?

Removes preventing the default behavior on the click event, to re-add support for ctrl/CMD + clicking on a link element to open the URL in a new tab.

Note: Angular's router link directive source code can be found here. There is no behavior that prevents the event, so I am uncertain why we did in the first place.

Does this introduce a breaking change?

• Yes
• No

Other information

Closing #24440 in favor of this PR (co-authored-by applied to the commit).

Cypress test support for CTRL/CMD + click is pretty tricky/finicky. Opted for not writing tests on this behavior at this time.

If we would like to support this behavior on our Ionic components in the future, we will likely need to render an a tag around the element or swap the inner element to an a tag to support the browser default behaviors.

[averyjohnston]

Gave this a shot with dev build 6.0.14-dev.11648653425.149f5ee0, works great 👍 I'm not sure why we had that preventDefault either haha. The only thing I could think of was maybe preventing hard refreshes in the page transition due to the href, but I'm still seeing the nice SPA transition. So yeah, looks good.

[liamdebeasi]

Do we still need the "@internal" comment?

[liamdebeasi]

Apologies to the user I just tagged by accident 😂

[sean-perkins]

I don't believe so. I'm unsure why it was marked as @internal in the first place. Not a convention that exists in the Angular code base for a similar implementation: https://github.com/angular/angular/blob/master/packages/router/src/directives/router_link.ts#L238-L239

Also not a method that developers are accessing through a public API.

[lincolnthree]

I believe this PR has introduced a regression where all routerLink elements now cause the entire page to reload on a normal mouse click or mobile tap (e.g. they break the single-page app paradigm). Working on a reproduction in the morning. I can't be sure it was this PR, but upgrading to 6.0.15 introduced the behavior, and rolling back to 6.0.14 has resolved the issue -- this patch seems closely related.

Just wanted to give y'all a heads up just in case, since this could affect a bunch of people if they hit the same issue that we did. More info after some sleep.

[liamdebeasi]

Thanks for the heads up. I can reproduce this and will work on a fix.

[lincolnthree]

@liamdebeasi Awesome, thank you. I won't bother with a repro then :) You are the best as always.

[liamdebeasi]

Mind giving this dev build a test?

6.0.16-dev.11649343768.1184e6cf

Pressing ion-button or ion-item with a routerLink should not cause a page refresh. Pressing an a with an href should cause a page refresh, and you should also be able to open those links in a new tab with a Ctrl/Cmd + Click.

[liamdebeasi]

This issue is resolved in Ionic 6.0.16. Please update your apps to receive a fix:

npm install @ionic/[email protected]