[MIRROR] Fix some Section jank disallowing stamps from working proper…

…ly and messengers from auto-scrolling (#923) (#1951)

* Fix some Section jank disallowing stamps from working properly and messengers from auto-scrolling (#81418)

## About The Pull Request

### Alternate title: "A spark of madness drove me to want to fix
paperwork tgui"

While doing paperwork I noticed that stamps could not be placed below a
certain point on paperwork, just reverting to being higher up. Actually
testing this, it seemed the stamps were not accounting for scroll offset
in some way.
<details>
  <summary>Images of Stamping Issue</summary>
  

![image](https://github.com/tgstation/tgstation/assets/42909981/96dbdafa-b7fc-44ed-8a93-2d0d9bfd51dc)

![image](https://github.com/tgstation/tgstation/assets/42909981/7193585a-511a-41b5-a3a0-8670ed692c14)

</details>

This led me to look into the code, and lo and behold, when
`PaperSheetStamper` in `interfaces/PaperSheet.tsx` was trying to get the
scroll offset from its `scrollableRef` it was simply returning 0 for
`scrollTop` and 469 for `height` regardless of what either value should
be.
I presumed _something_ was causing the reference to not be properly
forwarded from `PreviewView`'s `Section` to `PaperSheetStamper`, and
continued by testing `onScrollHandler`.

`onScrollHandler` seemed to actually have the desired values, and so I
made my first naive fix: having `PrimaryView` instead handle taking the
values from `onScrollHandler` and forwarding them to
`PaperSheetStamper`, and this worked fine!
However, I felt I didn't understand enough about _why_ the reference
method didn't work to commit to this approach.

So I went to look deeper, and found that there's actually at least one
more thing suffering from this issue.
`interfaces/NtosMessenger/ChatScreen.tsx`, the PDA messenger.
Specifically, its `scrollToBottom` method was using a `Section`
reference similarly to our problematic `interfaces/PaperSheet.tsx`, and
wasn't actually working.
<details>
  <summary>Images of PDA Issue</summary>
  

![image](https://github.com/tgstation/tgstation/assets/42909981/354c3ee5-15e2-4ffb-95eb-b4d33f58701a)

</details>
With tens of tabs of React documentation and tg files open went through
a crash course in React references, and hey uh- wait, no, hold on. It
can't be. Right?

`components/Section.tsx` was handling two distinct references,
`forwardedRef` and `contentRef`, in two distinct places. The former, at
the highest `div`, the latter at a lower `div`... together with
`onScroll`, seemingly confirming my suspicions.
Looking through the file's history, it seems that over the last several
prs and reimplementations of parts of this file, the `div` it was
forwarding a reference from was moved from being the same as `onScroll`
to a different `div` altogether. Then, this became `contentRef`.

And to confirm this, merging `forwardedRef` and `contentRef` seemed to
fix the issues with both stamping paperwork and PDA auto-scrolling just
fine.
<details>
  <summary>Images of PDA Issues Resolved</summary>
  

![image](https://github.com/tgstation/tgstation/assets/42909981/aaa6ccac-3271-4056-9761-a0a38ac72a4b)

![image](https://github.com/tgstation/tgstation/assets/42909981/1f03f627-8a75-4df2-89c2-757ff9fdbd42)

</details>

<details>
  <summary>Images of Stamping Issues Resolved</summary>
  

![image](https://github.com/tgstation/tgstation/assets/42909981/a4e36095-ec6b-4109-aeee-17cf9ced6ef3)

![image](https://github.com/tgstation/tgstation/assets/42909981/ca4eb62b-c7ed-4420-b869-58970da6a9f9)

![image](https://github.com/tgstation/tgstation/assets/42909981/bf1309ec-3a8b-4aa5-baed-b45a8ccfc577)

</details>

Final notes:
- I recognized the issue at play in a moment of madness at 1am, it
however still took an extra day for me to feel confident enough in my
React to actually fix this without crashing everything else ever.
- Coincidentally "Ace Combat Zero Soundtrack - Zero" was playing in the
background while finalizing this.
- For some esoteric reason beyond my current comprehension, as opposed
to non-stamped paperwork, stamped paperwork does not auto-focus when
scrolled over sometimes. As it already did this before my changes, I am
considering it outside of the scope of this pr. It would also most
likely deal 3d8 psychic damage, on top of it being 1:53am as of writing
this.
## Why It's Good For The Game

Reduces paperwork jank, but broadly fixes issues with Section's
forwarded reference not actually being a reference to the scrolling
part. This includes PDA messengers' scroll to bottom actually working.

Fixes #60508. Given the issue is from 2021 I don't think it's _related_,
but it exists again and hey it works fine now!
Fixes PDA messengers' scroll to bottom actually working.
Fixes, uh, probably other stuff that used to rely on this working.
## Changelog
:cl:
fix: Fixed stamps not accounting for scroll offset. You can actually
stamp paperwork properly without using accursed knowledge again.
fix: Fixed PDA messenger not scrolling to the bottom when a new message
gets sent.
/:cl:

* Fix some Section jank disallowing stamps from working properly and messengers from auto-scrolling

---------

Co-authored-by: NovaBot <[email protected]>
Co-authored-by: _0Steven <[email protected]>

tgui/packages/tgui/components/Section.tsx

@@ -5,7 +5,7 @@
  */
 
 import { canRender, classes } from 'common/react';
-import { forwardRef, ReactNode, RefObject, useEffect, useRef } from 'react';
+import { forwardRef, ReactNode, RefObject, useEffect } from 'react';
 
 import { addScrollableNode, removeScrollableNode } from '../events';
 import { BoxProps, computeBoxClassName, computeBoxProps } from './Box';
@@ -70,19 +70,18 @@ export const Section = forwardRef(
       ...rest
     } = props;
 
-    const contentRef = useRef<HTMLDivElement>(null);
-
     const hasTitle = canRender(title) || canRender(buttons);
 
     /** We want to be able to scroll on hover, but using focus will steal it from inputs */
     useEffect(() => {
-      if (!contentRef.current) return;
+      if (!forwardedRef?.current) return;
       if (!scrollable && !scrollableHorizontal) return;
 
-      addScrollableNode(contentRef.current);
+      addScrollableNode(forwardedRef.current);
 
       return () => {
-        removeScrollableNode(contentRef.current!);
+        if (!forwardedRef?.current) return;
+        removeScrollableNode(forwardedRef.current!);
       };
     }, []);
 
@@ -98,7 +97,6 @@ export const Section = forwardRef(
           computeBoxClassName(rest),
         ])}
         {...computeBoxProps(rest)}
-        ref={forwardedRef}
       >
         {hasTitle && (
           <div className="Section__title">
@@ -110,7 +108,9 @@ export const Section = forwardRef(
           <div
             className="Section__content"
             onScroll={onScroll}
-            ref={contentRef}
+            // For posterity: the forwarded ref needs to be here specifically
+            // to actually let things interact with the scrolling.
+            ref={forwardedRef}
           >
             {children}
           </div>