[Bug] Permalink to comment does not scroll properly to comment in image posts

[reallytiredofclowns]

My assumption is that a permalink to a comment should scroll to the comment in question when loading a post. For example, in this relatively massive thread of 500 comments, pasting this URL into my browser scrolls directly to the comment:

https://discuit.net/Discuit/post/a8Suq9qc/177aafefb113e2585abd3718

This appears to work also for link posts. This scrolls to the (current) last comment in the link post: https://discuit.net/DiscuitMeta/post/T6RfqFsG/17c31a51b2697b97b9ae9c30

It doesn't work for image posts, as far as I can tell. Examples:

https://discuit.net/Selfposting/post/peeuGQTq/17c1d84e85461b4f9c805316

https://discuit.net/Selfposting/post/gzvYGomi/17c39470fcdd737fc493ff7e

https://discuit.net/Memeskills/post/vLlIflik/17c2bbd5d303dde0d628e75a

My guess is this is due to the code not taking into account image height, as the erroneous offset seems smaller in pictures that are short.

[Codycody31]

This shouldn't be super hard to fix (meaning it is hard), maybe it's possible to get the height of the image and then ensure we take that into account when going to the comment? Once we know the height of the image, we should be able to just pass it down from ui/src/pages/Post/index.jsx->ui/src/pages/Post/CommentSection.jsx->ui/src/pages/Post/Comment.jsx.

discuit/ui/src/pages/Post/Comment.jsx

Lines 209 to 215 in 1ebf2d6

	useEffect(() => {
	if (focused && div.current) {
	const pos =
	document.documentElement.scrollTop + div.current.getBoundingClientRect().top - 100;
	window.scrollTo(0, pos);
	}
	}, [focused, focusId]);

Should fix

      if (imageHeight) {
        const pos = imageHeight + div.current.getBoundingClientRect().top - 500;
        window.scrollTo(0, pos);
      } else {
        const pos =
          document.documentElement.scrollTop + div.current.getBoundingClientRect().top - 100;
        window.scrollTo(0, pos);
      }

[noClaps]

Since each comment has its commentId in its id attribute, couldn't you just do something like a document.getElementById(commentId).scrollIntoView()? It would be a lot more consistent, although it would rely on the comment being loaded in first before it can be selected by getElementById().

[Codycody31]

Maybe. The way I wrote it works, however I can give what you said a try. I think the way it does it right now is so that it doesn't have to be rendered to focus on it. However I have no idea if that's true

[noClaps]

You could also try adding it to the URL hash, and that would scroll to it automatically. That wouldn't work for comments that get hidden behind "More comments" since the element with the id would have to be rendered in. Then again, I don't know if the current approach works for that either.

If my approach works I think it would be better than trying to guess where the comment would be.

[reallytiredofclowns]

Hmmm noClaps is correct; the "More Comments" overflow doesn't work with the existing codebase.

This comment is in a massive 500-comment post and doesn't scroll: https://discuit.net/general/post/ze0qhfVG/1779f233d1b6b17b04e62fca. Once the additional comment is loaded, it's highlighted as if it were focussed.

It's not because it's a deleted comment; that scrolls fine here: https://discuit.net/general/post/ze0qhfVG/1779e5b3c931ec6677930879

[Codycody31]

ah okay, what if we first did a check to ensure the comment exists, and if it does then just keep calling the API till we reach it? I believe that would solve the issue of not scrolling to it. And i'll give what noclaps said a try out, it seems it would make it actually appear properly each time

[reallytiredofclowns]

Hmmm, Reddit does this by directing comment permalinks to the local context. This comment, for example, is not in Reddit's initial page load, and the permalink doesn't go to the full post. Not sure if Discuit has the supporting architecture for this, though. This is turning into a nontrivial problem...

[Codycody31]

yeah, I don't believe we can be that selective currently