Repository: getsentry/sentry
Pull request: fix(replays): SplitPanel conditional handlers
### Changes
- Conditionally adds mouse event handlers to split panel if mousedown is engaged

Closes #36816 


<!--

  Sentry employees and contractors can delete or ignore the following.

-->

### Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.


Problem statement:
Bug: SplitPanel should disable mouse event handlers when the pointer is not pressed
Currently the `<SplitPanel>` calls `useMouseTracking()` which unconditionally  adds an `onMouseMove` handler (and others).

We don't need that handler being called unless the mouse button is actually being clicked. Therefore `useMouseTracking()` should have an option that toggles it on/off. When it's off the event handlers are removed from the ref, when it's on then the handlers are added again.
Then `SplitPanel` can pass in it's existing `mousedown` state value to control things!

Hints from issue discussion:

Commit message: fix(replays): conditionally add event handlers if mousedown

Patch to review:
diff --git a/static/app/views/replays/detail/layout/splitPanel.tsx b/static/app/views/replays/detail/layout/splitPanel.tsx
index dc7bf2341179d0..6171654d8d2738 100644
--- a/static/app/views/replays/detail/layout/splitPanel.tsx
+++ b/static/app/views/replays/detail/layout/splitPanel.tsx
@@ -197,11 +197,15 @@ function SplitPanel(props: Props) {
     onPositionChange: handlePositionChange,
   });
 
+  const activeTrackingProps = mousedown
+    ? mouseTrackingProps
+    : {ref: mouseTrackingProps.ref};
+
   if ('left' in props) {
     const {left: a, right: b} = props;
 
     return (
-      <SplitPanelContainer orientation="columns" size={sizeCSS} {...mouseTrackingProps}>
+      <SplitPanelContainer orientation="columns" size={sizeCSS} {...activeTrackingProps}>
         <Panel>{getValFromSide(a, 'content') || a}</Panel>
         <Divider
           slideDirection="leftright"
@@ -214,7 +218,7 @@ function SplitPanel(props: Props) {
   }
   const {top: a, bottom: b} = props;
   return (
-    <SplitPanelContainer orientation="rows" size={sizeCSS} {...mouseTrackingProps}>
+    <SplitPanelContainer orientation="rows" size={sizeCSS} {...activeTrackingProps}>
       <Panel>{getValFromSide(a, 'content') || a}</Panel>
       <Divider
         slideDirection="updown"


Code review comments:
diff_hunk: @@ -197,11 +197,15 @@ function SplitPanel(props: Props) {
     onPositionChange: handlePositionChange,
   });
 
+  const activeTrackingProps = mousedown
+    ? mouseTrackingProps
+    : {ref: mouseTrackingProps.ref};
path: static/app/views/replays/detail/layout/splitPanel.tsx
text: @user1:
why is it important that `ref` be included?

can you add a comment in the code about it

@author:
It's probably not actually needed really but my thought is the proper way if you're using a ref is to set it to avoid confusion on whether or not it will be available especially since changes aren't tracked. So the ref will be available like normal but don't add the event handlers while they aren't needed. I can add a comment tho for sure

@user1:
sounds like if we don't need it, then we won't even need a comment to explain. we can have nothing in this object instead