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

Progress Bar Persists on External Links with Specified target Attribute #25

Closed
johannbuscail opened this issue Feb 21, 2024 · 20 comments
Closed
Assignees
Labels
enhancement New feature or request

Comments

@johannbuscail
Copy link

Description

Hello, and first off, thank you for the amazing work on this project! I've encountered a consistent bug related to the behavior of the progress bar when interacting with external links.

Steps to Reproduce

  1. Navigate to a page that contains external links.
  2. Click on any external link where the <a> tag has a specified target attribute. This issue arises not only with target="_blank" but also with custom frame names.

Expected Behavior

The progress bar should complete or disappear after the external link has loaded, indicating that the navigation process has finished.

Actual Behavior

The progress bar appears and remains visible indefinitely, suggesting that the navigation or loading process is still ongoing. This occurs regardless of the target attribute specified in the <a> tag, affecting both standard _blank targets and custom frame names.

Links

https://www.w3schools.com/tags/att_a_target.asp

@ndungtse
Copy link
Owner

From my local testing links with target _blank doesn't show progress bar because they are ignored. May this can happen in frames because I didn't handle frames implementations. If you have a live example where a with _blank target attribute showing progressbar or code example you can show

@johannbuscail
Copy link
Author

Yes my bad, it doesnt show on target=_blank. We use a lot frames on our app.
Do you think this can be implemented easily ?

Maybe here, instead of checking if target equals _blank, check if it is not _self, _parent or _top

@ndungtse
Copy link
Owner

Yeah. I think I should those too. But because I haven’t used frames before I didn't explore other options but yes It can be implemented easily. So I'll do the same as it is on _blank target.

@ndungtse
Copy link
Owner

checkout latest release it tackles all target attribute values

@johannbuscail
Copy link
Author

Thanks for your quick reply !

This doesn't solve the framename issue. Framenames are basically like _blank but with a specific id.

It works in the following:

  1. It opens a new tab the first time the user clicks on the link.
  2. Then, it just switches to the tab if it's already opened.

Any string can be in the target attribute.

ex: <a target="editor">.

So the commit only partially solves the issue.

@ndungtse
Copy link
Owner

I think you can open another issue about frames compatibility as I think it is a potential feature to be worked on it carefully.
And thanks for your continuing contribution.

@johannbuscail
Copy link
Author

I don't think it must be another issue, as it is directly related to this one.

@johannbuscail
Copy link
Author

@ndungtse ?

@ndungtse
Copy link
Owner

This feature will be worked on and I'm gonna reopen the issue

@ndungtse ndungtse added the enhancement New feature or request label Feb 28, 2024
@ndungtse ndungtse reopened this Feb 28, 2024
@johannbuscail
Copy link
Author

johannbuscail commented Mar 8, 2024

I don't think this is quite complicated as it would only require to invert the if statement to return if anchor element is not on same page.
Instead of :

if (anchorElement.target === '_blank' || anchorElement.target === '_top' || anchorElement.target === '_parent')

It would only require:

if (anchorElement.target !== '_self' && anchorElement.target?.trim() !== "")
   return;

I'm not sure if the anchorElement.target?.trim() !== "" is needed. I don't know if not passing a target through html/jsx props automatically gives _self or if you need to handle it.

And for this:

const validAnchorELes = Array.from(anchorElements).filter((anchor) => {
if (anchor.href.startsWith('tel:+') || anchor.href.startsWith('mailto:')) return false;
return anchor.href && anchor.target !== '_blank';
});

Replace it to this:

const validAnchorELes = Array.from(anchorElements).filter((anchor) => { 
   if (anchor.href.startsWith('tel:+') || anchor.href.startsWith('mailto:')) return false; 
   if (anchor.target !== '_self' && anchor.target?.trim() !== "") // again not sure about the `anchor.target?.trim() !== ""`
       return false;
   return anchor.href; 
 }); 

@johannbuscail
Copy link
Author

@ndungtse Any update ?

@ndungtse
Copy link
Owner

Well, Sorry for oversight I was somehow busy but I tested it and will be available sonn

@johannbuscail
Copy link
Author

Thanks !

@MenroMi
Copy link

MenroMi commented May 10, 2024

Hello, I encountered the same problem when I wanted to go to another page using the next link, but in my case the links do not have any specific properties that can affect the progress bar.
I noticed that my problem only occurs when I try to go from the page of the new version of the project (was completed in next js 14 where i have app folder) to the page of the old version of the project (with the pages folder - next js 12). It seems that the progress bar sees the old project as an external page.

Screen.Recording.2023-12-11.at.10.37.09.mp4

@ndungtse
Copy link
Owner

ndungtse commented May 11, 2024

Hello, I encountered the same problem when I wanted to go to another page using the next link, but in my case the links do not have any specific properties that can affect the progress bar. I noticed that my problem only occurs when I try to go from the page of the new version of the project (was completed in next js 14 where i have app folder) to the page of the old version of the project (with the pages folder - next js 12). It seems that the progress bar sees the old project as an external page.
ad
Screen.Recording.2023-12-11.at.10.37.09.mp4

Well, I tried to go to the same site you developing and the issue didn't appear again. but I also noticed the full reload as you used default html a tag. did you manage to handle it after above comment? cause for me the issue didn't show up

record.mp4

@MenroMi
Copy link

MenroMi commented May 12, 2024

Hello, I encountered the same problem when I wanted to go to another page using the next link, but in my case the links do not have any specific properties that can affect the progress bar. I noticed that my problem only occurs when I try to go from the page of the new version of the project (was completed in next js 14 where i have app folder) to the page of the old version of the project (with the pages folder - next js 12). It seems that the progress bar sees the old project as an external page.
ad
Screen.Recording.2023-12-11.at.10.37.09.mp4

Well, I tried to go to the same site you developing and the issue didn't appear again. but I also noticed the full reload as you used default html a tag. did you manage to handle it after above comment? cause for me the issue didn't show up

record.mp4

Hello again. This problem doesn't appear when you use chrome, but if you check it in firefox / safari or opera, you can see it. At the moment I only have one solution, which is to add the target property to the link tag, but in general this does not solve the problem because the preloader disappears.
P.S I don't use the default html a tag, instead I use the next extended link component.

@ndungtse
Copy link
Owner

Thanks @MenroMi . I'm going to test it on those browsers and ckeck why it behaves like that

@ndungtse ndungtse added the help needed Calling contributors to fix the issue label May 20, 2024
@johannbuscail
Copy link
Author

Hello @ndungtse, it still doesn't work for me on custom targets. You only added the last part of my message but you haven't added this part on line 116:

I don't think this is quite complicated as it would only require to invert the if statement to return if anchor element is not on same page. Instead of :

if (anchorElement.target === '_blank' || anchorElement.target === '_top' || anchorElement.target === '_parent')

It would only require:

if (anchorElement.target !== '_self' && anchorElement.target?.trim() !== "")
   return;

@ndungtse
Copy link
Owner

ndungtse commented Aug 14, 2024

Hello @ndungtse, it still doesn't work for me on custom targets. You only added the last part of my message but you haven't added this part on line 116:

I don't think this is quite complicated as it would only require to invert the if statement to return if anchor element is not on same page. Instead of :

if (anchorElement.target === '_blank' || anchorElement.target === '_top' || anchorElement.target === '_parent')

It would only require:

if (anchorElement.target !== '_self' && anchorElement.target?.trim() !== "")
   return;

Ooh. I get it right now @johannbuscail . First I thought you need all target values to be evaluated, so as you suggested only target without '_self' and target with some value will not trigger the progressbar. I think that would be easier then 👍🏻. Gonna update it in next release

ndungtse added a commit that referenced this issue Aug 18, 2024
@ndungtse
Copy link
Owner

@johannbuscail check out new release I think issue should be now resolved

@ndungtse ndungtse removed the help needed Calling contributors to fix the issue label Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants