strongloop / strong-docs

build documentation sites for your node modules
Other
14 stars 3 forks source link

Broken permalinks for individual entities in TS docs #99

Closed bajtos closed 7 years ago

bajtos commented 7 years ago

Steps to reproduce:

shimks commented 7 years ago

This issue can be replicated in Chrome, but not in Safari nor Firefox. I do not know what that implies though lol.

virkt25 commented 7 years ago

@shimks that would mean that it's a Chrome specific issue, meaning they probably changed something under the hood that's breaking out script (and other browsers will likely follow suit sooner or later).

Might want to look at updating jQuery & jQuery.scrollTo since they're 4 years old and responsible for the scrolling. The new versions might have a fix in them for this issue.

raymondfeng commented 7 years ago

It seems to be a bug in Chrome. For example, we can start with https://github.com/prettier/prettier#cli and scroll down a bit. Refreshing doesn't move the page back to the anchor.

shimks commented 7 years ago

I've tried updating jQuery to the latest version (in the html file generated by Jekyll) and it doesn't seem to fix the problem.

~The broken permalinks/anchors seems to be a problem with every websites that uses anchorJs. Some of these include github and Jekyll. I've also tried updating anchorJs as well Link to anchor-js: https://github.com/bryanbraun/anchorjs~

I would like some input on what other think here.

shimks commented 7 years ago

I've identified the problem: there's a bug with Chrome where anchoring doesn't work upon refreshing.

One solution is to insert this hacky code into all html files that uses anchoring: https://stackoverflow.com/questions/38588346/anchor-a-tags-not-working-in-chrome-when-using. I was able to verify that this will work.

This should affect every parts of loopback.io, so off the top of my head the tools that need to be used to apply this fix is strongdocs and Jekyll in loopback.io. An alternative is to open an issue in Chromium and wait until they fix the issue.

@crandmck @raymondfeng Any thoughts?

crandmck commented 7 years ago

We could do this for http://loopback.io/doc/ and http://apidocs.loopback.io/ (probably in header.ejs, not strong-docs, btw), if we think it's important. It seems like a bit of a corner-case to me, because the anchors DO work when you initially click on the link. It's only if you reload the page that it doesn't work properly.

I don't feel that strongly about it one way or the other. If we do make this fix, we need to track when Chrome fixes the bug and remove the code at that point since it will then become superfluous.

shimks commented 7 years ago

I feel the same way about this. Maybe we could decide to apply the fix if someone feels strongly about making sure that the anchors stay in place when refreshed. Personally, I'd just be content with opening an issue with Chrome team and wait until they fix it.

shimks commented 7 years ago

Existing issues that document this bug are: https://bugs.chromium.org/p/chromium/issues/detail?id=738405 and https://bugs.chromium.org/p/chromium/issues/detail?id=602047

Considering that these issues have never been assigned to anyone nor triaged for a long time, I don't think we can expect the issue to be fixed by them any time soon.

Can anyone chip in their opinion on whether this issue is important enough to be addressed?

raymondfeng commented 7 years ago

Since it's a known bug in Chrome, I suggest that we defer the issue.

shimks commented 7 years ago

I'll close the issue since everyone's agreeing that it's not much of an issue. I can confirm that the anchoring works properly when the url with an anchor is visited from a new tab, and that's good enough for me.