Fix GitHub's buggy "scroll to focused comment" feature #3263

Test comment

This also happens for files in PRs, but there it's not as buggy, e.g. https://github.com/sindresorhus/refined-github/pull/3257/files#diff-b3df289d90dbc15afa339bff816f4838

This is impossible to solve because of the sticky header.

If you keep pressing it fast enough, you see all three behaviors happening.

• Fixed view with glitching.
• Fixed view without glitching.
• Incorrect view, ignoring the fix completely.

We can not say when we see each of these behaviors, except for adding smooth scroll ourselves, overriding GH's offsetting logic.

Yeah it looks like it's buggy because there are several pieces of code that try to change the scroll position in various event handlers. If we try to replace that logic, we have to implement all of it (e.g. there's logic that opens collapsed comments when they're :targeted and logic that starts loading the rest of the file in a diff when a specific line is :targeted)

Perhaps we can just find a way to fix https://github.com/sindresorhus/refined-github/pull/3242 since we control the URL changing. We need to do this without triggering hashchange:

• set location.hash
• trigger the :target style (<- this might not be possible)
• scroll the element into view

Perhaps we can just find a way to fix #3242 since we control the URL changing. We need to do this without triggering hashchange: ...

What we could do to avoid glitching in #3242:

1. scroll the element into the position where it is supposed to be
2. replace hash (this triggers :target, also hashchange)
3. GH will try to scroll page to where it is supposed to be

As of 3, browser in most cases should not do anything, because 3 happens immediately invalidating 2. Not an exact solution, but should avoid some amount of glitch.

Just realized I essentially explained everything GH does after hashchange 😅

I think this can be fixed "easily" by copying GitHub’s CSS and avoiding the :target selector altogether

• Copy GitHub’s CSS and adjust it:

-  .comment:target {
+  .comment.rgh-target {

• On anchor click:
  1. Use location.replace to set location.hash
  2. Use comment.scrollIntoView()
  3. Add rgh-target class to comment
• onhashchange / click on body:
  1. Remove rgh-target class
  2. Remove location.hash

I don't know if it's totally worth it, it might cause some conflicts 🤷‍♂️

Modifying hash in any way (using location.hash or location.replace) immediately triggers GH targeting behavior.

In Firefox, even where using scrollIntoView() to set comment in desired position and then changing hash causes it to glitch.

The only way to fix this I think would be to hijack GH's handler somehow.

Modifying hash in any way (using location.hash or location.replace) immediately triggers GH targeting behavior.

This doesn't:

history.replaceState({}, document.title, 'https://github.com/sindresorhus/refined-github/issues/3263#issuecomment-658685481')

However I think it's not worth getting into the trouble of recreating this behavior and very likely stumbling on conflicts and causing further bugs. Let's hope GitHub fixes it