fix: Sync-scrolling multiple panels stops at bottom of shortest panel when scrolling down

[mtrezza]

New Pull Request Checklist

• I am not disclosing a vulnerability.
• I am creating this PR in reference to an issue.

Issue Description

Sync-scrolling multiple panels stops at bottom of shortest panel when scrolling down.

Approach

Fix scroll position algorithm.

Summary by CodeRabbit

• Refactor
  • Improved event listener management in the data browser's multi-panel view, resulting in better performance and more reliable scroll synchronization when viewing multiple panels side-by-side.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[parse-github-assistant]

🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review.

[parseplatformorg]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors the multi-panel wrapper reference from React.createRef() to a callback ref, consolidating wheel event listener attachment and detachment logic within a dedicated class method. Updates all component lifecycle and rendering logic to use the new callback-based ref pattern.

Changes

Cohort / File(s)	Summary
Ref Management Refactor src/dashboard/Data/Browser/DataBrowser.react.js	Replaces React.createRef() with callback ref pattern for multi-panel wrapper; adds setMultiPanelWrapperRef() method to manage wheel event listener lifecycle; updates all references from multiPanelWrapperRef to multiPanelWrapperElement; binds callback in constructor; consolidates listener attach/detach logic in componentDidMount, componentDidUpdate, and componentWillUnmount

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Verify all instances of multiPanelWrapperRef are correctly replaced with multiPanelWrapperElement
• Confirm callback ref binding in constructor maintains proper this context
• Review wheel event listener attach/detach logic in setMultiPanelWrapperRef() to ensure proper cleanup
• Check lifecycle method changes (componentDidMount, componentDidUpdate, componentWillUnmount) correctly invoke listener management

Possibly related PRs

• feat: Add displaying multiple info panels and batch navigation #3020 — Adds multi-panel scroll handling and wheel listener lifecycle that this PR refactors to use the callback ref pattern for cleaner management.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the bug being fixed: sync-scrolling behavior stopping at the shortest panel when scrolling down, which matches the implementation changes to the scroll handling logic.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Warning

Tools execution failed with the following error:

Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (3)

src/dashboard/Data/Browser/DataBrowser.react.js (3)

172-183: Wheel listener lifecycle in setMultiPanelWrapperRef looks sound; consider centralizing listener toggling

Storing the wrapper element and always removing the previous listener before attaching to the new one is correct and prevents leaks when the DOM node changes. However, attachment/detachment is now split between this method and componentDidUpdate, which makes the lifecycle harder to reason about.

You could simplify by centralizing the “does this component currently need a wheel listener?” decision into a single helper (e.g. updateWrapperWheelListener()) that both setMultiPanelWrapperRef and componentDidUpdate call, so the add/remove rules live in one place.

---

247-252: Good unmount cleanup; optionally clear pending wheel timeout

Removing the wheel listener from multiPanelWrapperElement on unmount is correct and avoids leaks. One small follow‑up: if this.wheelTimeout is still pending when the component unmounts, you could clear it here to avoid running a timeout callback after unmount (even though it only mutates an instance field):

componentWillUnmount() {
  document.body.removeEventListener('keydown', this.handleKey);
  window.removeEventListener('resize', this.updateMaxWidth);
  if (this.multiPanelWrapperElement) {
    this.multiPanelWrapperElement.removeEventListener('wheel', this.handleWrapperWheel);
  }
+ if (this.wheelTimeout) {
+   clearTimeout(this.wheelTimeout);
+   this.wheelTimeout = null;
+ }
}

---

319-330: Avoid duplicating wheel-listener attach/detach logic between lifecycle and ref

This conditional correctly responds to changes in panelCount / syncPanelScroll and (re)attaches or removes the wheel listener as needed when state changes. However, similar add/remove logic already exists in setMultiPanelWrapperRef, so the responsibility is now split between two places.

To reduce complexity and the risk of future drift, consider:

• Having setMultiPanelWrapperRef only update this.multiPanelWrapperElement, with no listener logic, and
• Moving all listener management into a single helper invoked from componentDidMount / componentDidUpdate / componentWillUnmount.

That way, any future change in when the listener should be active only needs to be updated once.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7f27f34 and 7c5e73a.

📒 Files selected for processing (1)

• src/dashboard/Data/Browser/DataBrowser.react.js (4 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Node 18
• GitHub Check: Docker linux/amd64

🔇 Additional comments (1)

src/dashboard/Data/Browser/DataBrowser.react.js (1)

1453-1457: Callback ref usage for multiPanelWrapper is correct and aligns with new helper

Switching the wrapper’s ref to this.setMultiPanelWrapperRef is consistent with the new multiPanelWrapperElement field and ensures you always have the live DOM element when the multi‑panel container mounts/unmounts. This integrates cleanly with the wheel‑listener lifecycle you’ve added.

[parseplatformorg]

🎉 This change has been released in version 8.1.0-alpha.4