fix(angular RouterLinkDelegateDirective): Make routerLink respect target and keyboard modifiers#28976
fix(angular RouterLinkDelegateDirective): Make routerLink respect target and keyboard modifiers#28976j-oppenhuis wants to merge 1 commit into
Conversation
…ier is used in RouterLinkDelegateDirective
thetaPC
left a comment
thetaPC
left a comment
There was a problem hiding this comment.
Thank you for submitting a PR to the project! We appreciate your contribution.
There are a few things that need to be addressed before we can move forward with the PR.
- Command clicking is not following expected browser behavior. While I’m able to
CMD+clickon the button and the link opens in a new tab, the active page navigates to the same url. - The
OPTIONkey is not being taken into account when clicking on the button if the user is using a Mac. The user shouldn't be able to open the link in a new tab and the page should not redirect to the link passed torouterLink. - As a noted in the first item, the page should not redirect to the link passed to
routerLink. The redirection is happening because of a behavior that is implemented inion-button. We would want to prevent Ionic's click listener from triggering when a new tab is opened. - We also want to check the operating system of the user to tie the correct key to the correct action. The
OPTIONkey should only be taken into account when the user is using a Mac.
Let me know if you have any questions! Thank you for your time and effort!
|
Thank you for your feedback. I understand that it is more complex than my initial idea. Instead of changing routerLink functionality, we should probably change the |
|
I have looked into the code and I think my suggestion is incorrect. But it seems we are not able to prevent the button or item navigation by using stoppropagation. The only thing I can think of is adding a |
|
We've checked out the problem, and it's a tough one. But because we're short on time, our team can't really dig into it right now. However, we're up for bouncing around ideas together. We might not have all the answers, but we'll do what we can to help out. I agree that passing an operating system is not the way to go. We want |
|
Hey, thanks for your PR! We've addressed this issue in another PR ( #31230 ), but have given you co-author credit for it thanks to your work on this one. This issue's fix will be deployed in the next release of Ionic Framework. |
Issue number: resolves ionic-team#26394 --------- ## What is the current behavior? When `routerLink` is applied to a non-anchor Ionic component (`ion-item`, `ion-button`), `RouterLinkDelegateDirective` always routes in-app on click. A ctrl/meta/shift/alt click, or a host with `target="_blank"`, gets swallowed by the in-app navigation, so the browser never opens the link in a new tab. A plain `<a routerLink>` honors these intents, but the Ionic delegate doesn't, even though these components render a native anchor in their shadow DOM that's capable of the native new-tab behavior. ## What is the new behavior? `RouterLinkDelegateDirective` now adds a capture-phase click listener so it runs before Angular's `RouterLink` handler and the directive's own bubble-phase `onClick`. When the click should be handled natively (a ctrl/meta/shift/alt modifier is held, or the host `target` is set to anything other than `_self`), it calls `stopImmediatePropagation()` to cancel the in-app navigation but leaves `preventDefault` alone, so the shadow-DOM anchor can still open the tab. This mirrors the modifier set Angular's own `RouterLink` guards on. Normal clicks are unaffected and continue to navigate in-app. The listener is removed in `ngOnDestroy`. ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information This supersedes ionic-team#28976 by @j-oppenhuis, which first surfaced the target and modifier-click gap. Credited as co-author below. To verify manually in the Angular preview, ctrl/cmd-click the two `ion-item` rows (a plain one and a `target="_blank"` one) and confirm each opens a new tab instead of navigating in place: https://ionic-framework-git-fix-angular-routerlink-modifi-896c58-ionic1.vercel.app/angular/standalone/router-link If you compare that against the main version of the same component, you'll be able to see how it's fixed: https://ionic-framework-git-main-ionic1.vercel.app/angular/standalone/router-link Co-authored-by: Jelle Oppenhuis <jelle@siyou.nl>
3 participants
Issue number: resolves #26394
What is the current behavior?
This fixes using routerLink in combination with a target different than _self. It also fixes a bug where CTRL + click or shift + click does not work like expected in combination with routerLink in Angular.
What is the new behavior?
When using a target other than _self and keyboard modifiers we are now stopping execution of navigating by routeDirection. This is not needed because the app will reload either way.
Does this introduce a breaking change?
Other information