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

Links in scully blogs do not respect Ctrl Click #1662

Open
hisham opened this issue May 9, 2023 · 5 comments
Open

Links in scully blogs do not respect Ctrl Click #1662

hisham opened this issue May 9, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@hisham
Copy link

hisham commented May 9, 2023

🐞 Bug report

Description

It seems for blog posts, scully takes over onclick behaviour in links, and if scully is aware of the route, it overrides the link navigation via this.router.navigate.

However, this means that ctrl-click to open link in new tab is not respected. target="_blank" is not respected either. Also I notice sometimes when user presses back, their location in the article does not remain the same either.

The relevant code is here:

async upgradeToRoutelink(elm: HTMLElement) {

🔬 Minimal Reproduction

Create angular scully app with blog post that links to a route in the app that scully is aware of.

💻Your Environment

Angular Version:
15

Scully Version:
2.1.41

@hisham hisham added the bug Something isn't working label May 9, 2023
@hisham
Copy link
Author

hisham commented May 9, 2023

Is this upgradeToRouteLink feature needed anymore btw? My links seem to work fine without it, and does not reload the page if I link within the angular app.

@hisham
Copy link
Author

hisham commented May 9, 2023

related to #1615 as well. I think there should be an Input to disable the scully routing upgrade for all links...

@SanderElias
Copy link
Contributor

@hisham, without the upgrade, your links don't stop working. However, they will do a full-page refresh navigation.
I'm unsure that if you use a [route-link]='somelink' one can do the right click either?

@hisham
Copy link
Author

hisham commented May 10, 2023

Hi @SanderElias - thanks for the response. Ok, yes I do see the page refresh navigation when I remove Scully's onclick handler. While not having page refresh is ideal, to be honest, it's not so bad.

To answer your second question, yes angular's routerLink directive does respect ctrl-click. Looks like they supported this since 2016 - see KiaraGrouwstra/angular@1ac9dda for the fix they implemented to support this. They catch the mouse event and if meta or ctrl key was pressed at same time, they let the browser do its thing and do not handle anything. So I guess Scully can do that as well maybe. Notice as well they look at the target attribute as well.

However, another issue I found is when I click on one of the links in the blog post and then go back, the scroll position in the original blog post page is not preserved. I haven't spent the time investigating why but I suspect it's due to how the scully-content component dynamically inserts blog content.

@hisham
Copy link
Author

hisham commented May 10, 2023

yeah I'm pretty sure scroll positioning is caused by scully's dynamic blog post content insertion. It seems to be done in a non-angular way, so maybe that's why angular is unable to preserve scroll history for blog posts. Scully might need to remember scroll positions itself like what is proposed here: https://dev.to/johncarroll/angular-how-to-refresh-scroll-position-on-back-5e1b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants