sindresorhus / hide-files-on-github

Chrome extension - Hide nonessential files from the GitHub file browser
https://chrome.google.com/webstore/detail/hide-files-on-github/lpnakhpaodhdkleejaehlapdhbgjbddp
MIT License
320 stars 36 forks source link

Support pjax updates #63

Closed fregante closed 7 years ago

fregante commented 7 years ago

I think this happens when GitHub serves an outdated HTML files and then fetches the recent content. Perhaps when you visit a repo that has changed since your last visit.

Here you can see me visiting such a repo:

before

And here's after this PR:

after

The good thing is that if you find a repo with this behavior, it's easy to test: you can go back and forth in history and it will always behave the same, allowing you to reload this extension between tries.

juliangruber commented 7 years ago

a simpler trigger:

window.addEventListener('pjax:complete',() => update(settings))

(from https://github.com/juliangruber/chrome-github-expanded/blob/master/index.js#L24)

fregante commented 7 years ago

I don't think that's enough in this case since the existing pjax:end is not fired either. Does pjax:complete work after navigating back and forth? From pjax's docs it doesn't sound like it does (although they're not very clear)

fregante commented 7 years ago

Tested, confirmed: pjax:complete does not fire at all on history events.

window.addEventListener('pjax:complete', console.log);
juliangruber commented 7 years ago

You're right that pjax:complete doesn't always fire. Can you point us to a repo where this is reproducable?

fregante commented 7 years ago

@juliangruber I can't point you anywhere because the cache probably depends on you. You'll have to find a repo that you haven't visited in a while and hope to find the bug with the current version. Then go back and forth to reproduce it and switch to this PR's version to see it fixed.

fregante commented 7 years ago

This PR initially addressed the issue when loading a repo's homepage straight up, but it didn't fix it when you:

  1. Visit a repo's Issues page
  2. Go to the Code tab
  3. Let include-fragment update the file list

... because that didn't run the select('include-fragment.file-wrap') on the new pjax-loaded DOM.

I think this is now fixed, but can't quite test it because in this case the new fragment will persist between page changes.