fix: Only update scroll position when unmounting the editor by Its-treason · Pull Request #6420 · usebruno/bruno

Its-treason · 2025-12-15T23:35:56Z

Description

When scrolling, the responsePaneScrollPosition was updated on every frame, causing unnecessary re-renders of everything related to the tab, including the code editor itself. This caused performance issues and made scrolling look "sluggish".

The scroll location is now only saved when needed (on unmount/navigation) rather than on every scroll event. The scroll position is still properly restored after sending a new request.

I also added proper cleanup for the CodeMirror instance. This prevents duplicate editors in development mode due to React StrictMode's double-rendering: https://react.dev/reference/react/StrictMode#fixing-bugs-found-by-double-rendering-in-development (Notice the doubled scrollbar in the before video)

const wrapper = this.editor.getWrapperElement();
wrapper?.parentNode?.removeChild(wrapper);

Before

2025-12-16.00-27-03.mp4

After

2025-12-16.00-27-55.mp4

Notice in the "after" video, there are no red bars in the performance recording. These red bars indicate dropped frames.

Summary by CodeRabbit

Refactor
- Removed continuous scroll listener and its handler from the editor to simplify event processing.
- Now the editor reports final scroll state when it is closed and its DOM wrapper is removed to ensure thorough cleanup.
- Retained cleanup for link-awareness and lint tooltips to preserve stability.

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

coderabbitai · 2025-12-15T23:36:08Z

Walkthrough

Removed the CodeEditor internal scroll handler and its event binding/unbinding. componentWillUnmount now directly invokes the provided onScroll prop with the editor instance and removes the editor wrapper from the DOM.

Changes

Cohort / File(s)	Summary
CodeEditor scroll event refactoring `packages/bruno-app/src/components/CodeEditor/index.js`	Removed `editor.on('scroll', this.onScroll)` and the `onScroll` method; removed corresponding `editor.off('scroll', this.onScroll)` cleanup; in `componentWillUnmount` directly call `this.props.onScroll(this.editor)` and remove the editor wrapper from the DOM; retained other cleanup (link awareness, lint tooltip).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Verify the direct prop invocation in componentWillUnmount receives the intended editor state (e.g., current scrollTop) and the prop is defined before calling.
Confirm removing the event listener does not break any scroll-sync features or assumptions elsewhere.
Ensure DOM removal order (calling the callback vs removing wrapper) cannot cause access to a detached DOM node.

Suggested labels

size/S

Suggested reviewers

helloanoop
lohit-bruno
naman-bruno
bijin-bruno
Pragadesh44-Bruno

Poem

The scroll handler quietly withdrew,
A direct goodbye the editor knew,
Less glue, less noise, a cleaner view—
Code rests tidy, ready and new. ✨

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 accurately describes the main change: stopping scroll position updates on every frame and only saving it during unmount, which directly addresses the performance regression mentioned in the PR objectives.
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

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b10ef5c and 44b9df6.

📒 Files selected for processing (1)

packages/bruno-app/src/components/CodeEditor/index.js (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions. () => {} is good
No space between function name and parentheses. func() not func ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly

Files:

packages/bruno-app/src/components/CodeEditor/index.js

🧬 Code graph analysis (1)

packages/bruno-app/src/components/CodeEditor/index.js (1)

packages/bruno-app/src/utils/codemirror/lint-errors.js (1)

wrapper (86-86)

⏰ 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). (6)

GitHub Check: Playwright E2E Tests
GitHub Check: CLI Tests
GitHub Check: SSL Tests - macOS
GitHub Check: SSL Tests - Windows
GitHub Check: Unit Tests
GitHub Check: SSL Tests - Linux

🔇 Additional comments (2)

packages/bruno-app/src/components/CodeEditor/index.js (2)

287-288: LGTM!

The cleanup code properly removes the CodeMirror wrapper element from the DOM on unmount, preventing duplicate editors in React StrictMode. The optional chaining guards against null references, and the pattern is consistent with existing codebase usage.

277-279: Guard check correctly implemented; callback signature is backward compatible.

The guard check prevents calling an undefined prop. The signature change from { doc: { scrollTop } } to the full this.editor instance is backward compatible—CodeMirror editor instances expose doc.scrollTop, so parent components accessing event.doc.scrollTop continue to work without issues. This change is actually a compatible enhancement offering greater flexibility.

Likely an incorrect or invalid review comment.

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

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

coderabbitai

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a9c63e6 and b10ef5c.

📒 Files selected for processing (1)

packages/bruno-app/src/components/CodeEditor/index.js (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions. () => {} is good
No space between function name and parentheses. func() not func ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly

Files:

packages/bruno-app/src/components/CodeEditor/index.js

🧬 Code graph analysis (1)

packages/bruno-app/src/components/CodeEditor/index.js (1)

packages/bruno-app/src/utils/codemirror/lint-errors.js (1)

wrapper (86-86)

⏰ 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). (6)

GitHub Check: SSL Tests - Linux
GitHub Check: CLI Tests
GitHub Check: Unit Tests
GitHub Check: Playwright E2E Tests
GitHub Check: SSL Tests - Windows
GitHub Check: SSL Tests - macOS

🔇 Additional comments (1)

packages/bruno-app/src/components/CodeEditor/index.js (1)

285-286: Excellent cleanup implementation.

The CodeMirror wrapper removal is properly implemented with optional chaining and matches the established pattern in the codebase. This cleanup is essential for preventing duplicate editors when React StrictMode remounts components during development.

packages/bruno-app/src/components/CodeEditor/index.js

before the scroll position was updatet on every scroll, causing everything related to the tab to rerender.

Its-treason requested review from bijin-bruno, helloanoop, lohit-bruno and naman-bruno as code owners December 15, 2025 23:35

pull-request-size bot added the size/XS label Dec 15, 2025

Its-treason force-pushed the fix/code-editor-scroll-performance branch from b10ef5c to feebb1c Compare December 15, 2025 23:37

coderabbitai bot reviewed Dec 15, 2025

View reviewed changes

packages/bruno-app/src/components/CodeEditor/index.js Outdated Show resolved Hide resolved

fix: Only update scroll position when unmounting the editor

44b9df6

before the scroll position was updatet on every scroll, causing everything related to the tab to rerender.

Its-treason force-pushed the fix/code-editor-scroll-performance branch from feebb1c to 44b9df6 Compare December 15, 2025 23:40

pull-request-size bot added size/S and removed size/XS labels Dec 15, 2025

bijin-bruno approved these changes Dec 16, 2025

View reviewed changes

bijin-bruno merged commit 2eb8db9 into usebruno:main Dec 16, 2025
9 of 11 checks passed

coderabbitai bot mentioned this pull request Feb 21, 2026

feat(request-pane): restore body tab scroll position on tab switch #7250

Open

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Only update scroll position when unmounting the editor#6420

fix: Only update scroll position when unmounting the editor#6420
bijin-bruno merged 1 commit intousebruno:mainfrom
Its-treason:fix/code-editor-scroll-performance

Its-treason commented Dec 15, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 15, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Its-treason commented Dec 15, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Before

After

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested labels

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Its-treason commented Dec 15, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 15, 2025 •

edited

Loading