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

Does not trigger HTML insertion on SPA-like navigation #13

Closed
jcbhmr opened this issue Jul 2, 2022 · 7 comments
Closed

Does not trigger HTML insertion on SPA-like navigation #13

jcbhmr opened this issue Jul 2, 2022 · 7 comments

Comments

@jcbhmr
Copy link

jcbhmr commented Jul 2, 2022

GitHub uses an SPA-like navigation system which does magic ✨ to avoid triggering a full page reload. This means that the HTML injection that is done once doesn't happen again when a non-navigation-but-still-sorta-navigation event occurs when the user changes "tabs" in the repo.

Video of what I am talking about:
https://user-images.githubusercontent.com/61068799/177011417-7976339c-1971-46e6-b4f3-b3246bfbd4fb.mp4

Listening for some kind of URL/history/navigation-related event and re-triggering the init() call might fix it? I don't really know.

Maybe relevant links:

There looks to be like 4 more StackOverflow questions basically asking the same thing and all getting basic: "you monkeypatch the history.pushState() method" answers:

@jcbhmr
Copy link
Author

jcbhmr commented Jul 2, 2022

Possible solution? Haven't tested it.

diff --git a/src/js/main.js b/src/js/main.js
index c62ef70..4f44dd0 100644
--- a/src/js/main.js
+++ b/src/js/main.js
@@ -245,3 +245,13 @@ function fixCommitsPageStyle (menuElement) {
 
 window.onload = init;
 init();
+
+// Caniuse: https://caniuse.com/mdn-api_navigation
+// Chrome dev article: https://developer.chrome.com/docs/web-platform/navigation-api/#try-the-navigation-api:~:text=navigation.addEventListener(%27navigatesuccess%27%2C%20event%20%3D%3E%20%7B%0A%20%20loadingIndicator.hidden%20%3D%20true%3B%0A%7D)%3B
+try {
+  navigation.addEventListener('navigatesuccess', event => {
+    init()
+  });
+} catch {
+  // IDK what to do for a fallback
+}

@zvizvi
Copy link
Owner

zvizvi commented Jul 2, 2022

Hi @jcbhmr.
Thank you for opening this issue.
What I did specifically on the GitHub site is listen to the custom event pjax:end which is triggered when the page route changes.
This is a good solution only for the GitHub site, as it uses the 'jquery-pjax' library.

document.addEventListener('pjax:end', () => {

@zvizvi
Copy link
Owner

zvizvi commented Jul 2, 2022

Update:
They seem to have stopped using 'jquery-pjax' recently, so this solution has actually stopped working...
Indeed, other solutions need to be sought 🤔

@jcbhmr
Copy link
Author

jcbhmr commented Jul 4, 2022

EDIT: Even though it looked like a Chrome-only extension at first glance from the description/link at the top, I double-checked and now see that it has a Firefox counterpart. This means this solution WON'T work.

I assume this is a Chrome-only extension? If so, how far back do you want to support? If its Chrome 102+, then the onnavigatesuccess is a perfect drop-in substitute for that already existing listener! (I think)

If not, IDK what to do...

Thoughts?

Can I use link: https://caniuse.com/mdn-api_navigation
image

@jcbhmr
Copy link
Author

jcbhmr commented Jul 4, 2022

Another thing you could use as an indicator is a MutationObserver tied to the progress bar that GitHub uses to signal SPA loading... IDK if that is a good idea though.

image
image

It might not even be that big of a performance hit since you can trim down what the browser watches for to be only direct children: new MutationObserver(f).observe(document.documentElement, { childList: true, subtree: false })

@jcbhmr
Copy link
Author

jcbhmr commented Jul 12, 2022

I went digging for GitHub-specific events that they use. Turns out there is a collection of soft-nav:* events that seem very similar to the pjax:* events that used to be used.

image

I added a listener to the soft-nav:success event, and it seems to be fired at the same time the navigatesuccess event would usually be triggered. Maybe this is a drop-in replacement for the old code, just with a new event name?

zvizvi added a commit that referenced this issue Jul 30, 2022
@zvizvi
Copy link
Owner

zvizvi commented Jul 30, 2022

Fixed now in v2.0.3, using the soft-nav:success event listener.
Thank you @jcbhmr!

@zvizvi zvizvi closed this as completed Jul 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants